[PATCH v3] ACPI: NFIT: Use cleanup.h helpers instead of devm_*()
The new cleanup.h facilities that arrived in v6.5-rc1 can replace the
the usage of devm semantics in acpi_nfit_init_interleave_set(). That
routine appears to only be using devm to avoid goto statements. The
new __free() annotation at variable declaration time can achieve the same
effect more efficiently.
There is no end user visible side effects of this patch, I was
motivated to send this cleanup to practice using the new helpers.
Suggested-by: Dave Jiang
Suggested-by: Andy Shevchenko
Reviewed-by: Dave Jiang
Reviewed-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
Dan, would you like me to give you credit for the changelog changes
with Co-developed-by tag ?
v3:
- changed changelog
v2:
- removed first commit from the patchset, as the commit couldn't
be marked as a fix
- squashed those commits together, since the second one were
mostly overwriting the previous one
drivers/acpi/nfit/core.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 3826f49d481b..67a844a705c4 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2257,26 +2257,23 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
struct nd_region_desc *ndr_desc,
struct acpi_nfit_system_address *spa)
{
+ u16 nr = ndr_desc->num_mappings;
+ struct nfit_set_info2 *info2 __free(kfree) =
+ kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ struct nfit_set_info *info __free(kfree) =
+ kcalloc(nr, sizeof(*info), GFP_KERNEL);
struct device *dev = acpi_desc->dev;
struct nd_interleave_set *nd_set;
- u16 nr = ndr_desc->num_mappings;
- struct nfit_set_info2 *info2;
- struct nfit_set_info *info;
int i;
+ if (!info || !info2)
+ return -ENOMEM;
+
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
if (!nd_set)
return -ENOMEM;
import_guid(_set->type_guid, spa->range_guid);
- info = devm_kcalloc(dev, nr, sizeof(*info), GFP_KERNEL);
- if (!info)
- return -ENOMEM;
-
- info2 = devm_kcalloc(dev, nr, sizeof(*info2), GFP_KERNEL);
- if (!info2)
- return -ENOMEM;
-
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = _desc->mapping[i];
struct nvdimm *nvdimm = mapping->nvdimm;
@@ -2337,8 +2334,6 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
}
ndr_desc->nd_set = nd_set;
- devm_kfree(dev, info);
- devm_kfree(dev, info2);
return 0;
}
--
2.41.0
[PATCH v2] ACPI: NFIT: Fix local use of devm_*()
devm_*() family of functions purpose is managing memory attached to a
device. So in general it should only be used for allocations that should
last for the whole lifecycle of the device. This is not the case for
acpi_nfit_init_interleave_set(). There are two allocations that are only
used locally in this function.
Fix this by switching from devm_kcalloc() to kcalloc(), and adding
modern scope based rollback. This is similar to C++ RAII and is
preferred way for handling local memory allocations.
Reported-by: Andy Shevchenko
Suggested-by: Dave Jiang
Suggested-by: Andy Shevchenko
Reviewed-by: Dave Jiang
Reviewed-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
v2:
- removed first commit from the patchset, as the commit couldn't
be marked as a fix
- squashed those commits together, since the second one were
mostly overwriting the previous one
drivers/acpi/nfit/core.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 3826f49d481b..67a844a705c4 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2257,26 +2257,23 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
struct nd_region_desc *ndr_desc,
struct acpi_nfit_system_address *spa)
{
+ u16 nr = ndr_desc->num_mappings;
+ struct nfit_set_info2 *info2 __free(kfree) =
+ kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ struct nfit_set_info *info __free(kfree) =
+ kcalloc(nr, sizeof(*info), GFP_KERNEL);
struct device *dev = acpi_desc->dev;
struct nd_interleave_set *nd_set;
- u16 nr = ndr_desc->num_mappings;
- struct nfit_set_info2 *info2;
- struct nfit_set_info *info;
int i;
+ if (!info || !info2)
+ return -ENOMEM;
+
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
if (!nd_set)
return -ENOMEM;
import_guid(_set->type_guid, spa->range_guid);
- info = devm_kcalloc(dev, nr, sizeof(*info), GFP_KERNEL);
- if (!info)
- return -ENOMEM;
-
- info2 = devm_kcalloc(dev, nr, sizeof(*info2), GFP_KERNEL);
- if (!info2)
- return -ENOMEM;
-
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = _desc->mapping[i];
struct nvdimm *nvdimm = mapping->nvdimm;
@@ -2337,8 +2334,6 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
}
ndr_desc->nd_set = nd_set;
- devm_kfree(dev, info);
- devm_kfree(dev, info2);
return 0;
}
--
2.41.0
[PATCH v3 6/6] ACPI: NFIT: Remove redundant call to to_acpi_dev()
In acpi_nfit_ctl() ACPI handle is extracted using to_acpi_dev()
function and accessing handle field in ACPI device. After transformation
from ACPI driver to platform driver this is not optimal anymore. To get
a handle it's enough to just use ACPI_HANDLE() macro to extract the
handle.
Suggested-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 6b9d10cae92c..402bb56d4163 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -475,8 +475,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
guid = to_nfit_uuid(nfit_mem->family);
handle = adev->handle;
} else {
- struct acpi_device *adev = to_acpi_dev(acpi_desc);
-
cmd_name = nvdimm_bus_cmd_name(cmd);
cmd_mask = nd_desc->cmd_mask;
if (cmd == ND_CMD_CALL && call_pkg->nd_family) {
@@ -493,7 +491,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
guid = to_nfit_uuid(NFIT_DEV_BUS);
}
desc = nd_cmd_bus_desc(cmd);
- handle = adev->handle;
+ handle = ACPI_HANDLE(dev);
dimm_name = "bus";
}
--
2.41.0
[PATCH v3 5/6] ACPI: NFIT: Replace acpi_driver with platform_driver
NFIT driver uses struct acpi_driver incorrectly to register itself.
This is wrong as the instances of the ACPI devices are not meant
to be literal devices, they're supposed to describe ACPI entry of a
particular device.
Use platform_driver instead of acpi_driver. In relevant places call
platform devices instances pdev to make a distinction with ACPI
devices instances.
NFIT driver uses devm_*() family of functions extensively. This change
has no impact on correct functioning of the whole devm_*() family of
functions, since the lifecycle of the device stays the same. It is still
being created during the enumeration, and destroyed on platform device
removal.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 3826f49d481b..6b9d10cae92c 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include "intel.h"
@@ -98,7 +99,7 @@ static struct acpi_device *to_acpi_dev(struct acpi_nfit_desc
*acpi_desc)
|| strcmp(nd_desc->provider_name, "ACPI.NFIT") != 0)
return NULL;
- return to_acpi_device(acpi_desc->dev);
+ return ACPI_COMPANION(acpi_desc->dev);
}
static int xlat_bus_status(void *buf, unsigned int cmd, u32 status)
@@ -3284,11 +3285,11 @@ static void acpi_nfit_put_table(void *table)
static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_device *adev = data;
+ struct device *dev = data;
- device_lock(>dev);
- __acpi_nfit_notify(>dev, handle, event);
- device_unlock(>dev);
+ device_lock(dev);
+ __acpi_nfit_notify(dev, handle, event);
+ device_unlock(dev);
}
static void acpi_nfit_remove_notify_handler(void *data)
@@ -3329,11 +3330,12 @@ void acpi_nfit_shutdown(void *data)
}
EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
-static int acpi_nfit_add(struct acpi_device *adev)
+static int acpi_nfit_probe(struct platform_device *pdev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_nfit_desc *acpi_desc;
- struct device *dev = >dev;
+ struct device *dev = >dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct acpi_table_header *tbl;
acpi_status status = AE_OK;
acpi_size sz;
@@ -3360,7 +3362,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
acpi_desc = devm_kzalloc(dev, sizeof(*acpi_desc), GFP_KERNEL);
if (!acpi_desc)
return -ENOMEM;
- acpi_nfit_desc_init(acpi_desc, >dev);
+ acpi_nfit_desc_init(acpi_desc, dev);
/* Save the acpi header for exporting the revision via sysfs */
acpi_desc->acpi_header = *tbl;
@@ -3391,7 +3393,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
return rc;
rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
-acpi_nfit_notify, adev);
+acpi_nfit_notify, dev);
if (rc)
return rc;
@@ -3475,11 +3477,11 @@ static const struct acpi_device_id acpi_nfit_ids[] = {
};
MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
-static struct acpi_driver acpi_nfit_driver = {
- .name = KBUILD_MODNAME,
- .ids = acpi_nfit_ids,
- .ops = {
- .add = acpi_nfit_add,
+static struct platform_driver acpi_nfit_driver = {
+ .probe = acpi_nfit_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .acpi_match_table = acpi_nfit_ids,
},
};
@@ -3517,7 +3519,7 @@ static __init int nfit_init(void)
return -ENOMEM;
nfit_mce_register();
- ret = acpi_bus_register_driver(_nfit_driver);
+ ret = platform_driver_register(_nfit_driver);
if (ret) {
nfit_mce_unregister();
destroy_workqueue(nfit_wq);
@@ -3530,7 +3532,7 @@ static __init int nfit_init(void)
static __exit void nfit_exit(void)
{
nfit_mce_unregister();
- acpi_bus_unregister_driver(_nfit_driver);
+ platform_driver_unregister(_nfit_driver);
destroy_workqueue(nfit_wq);
WARN_ON(!list_empty(_descs));
}
--
2.41.0
[PATCH v3 4/6] ACPI: AC: Rename ACPI device from device to adev
Since transformation from ACPI driver to platform driver there are two
devices on which the driver operates - ACPI device and platform device.
For the sake of reader this calls for the distinction in their naming,
to avoid confusion. Rename device to adev, as corresponding
platform device is called pdev.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 1dd4919be7ac..2618a7ccc11c 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -121,11 +121,11 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
{
struct device *dev = data;
struct acpi_ac *ac = dev_get_drvdata(dev);
- struct acpi_device *device = ACPI_COMPANION(dev);
+ struct acpi_device *adev = ACPI_COMPANION(dev);
switch (event) {
default:
- acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
+ acpi_handle_debug(adev->handle, "Unsupported event [0x%x]\n",
event);
fallthrough;
case ACPI_AC_NOTIFY_STATUS:
@@ -142,11 +142,11 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
msleep(ac_sleep_before_get_state_ms);
acpi_ac_get_state(ac);
- acpi_bus_generate_netlink_event(device->pnp.device_class,
+ acpi_bus_generate_netlink_event(adev->pnp.device_class,
dev_name(dev),
event,
ac->state);
- acpi_notifier_call_chain(device, event, ac->state);
+ acpi_notifier_call_chain(adev, event, ac->state);
kobject_uevent(>charger->dev.kobj, KOBJ_CHANGE);
}
}
@@ -205,7 +205,7 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
static int acpi_ac_probe(struct platform_device *pdev)
{
- struct acpi_device *device = ACPI_COMPANION(>dev);
+ struct acpi_device *adev = ACPI_COMPANION(>dev);
struct power_supply_config psy_cfg = {};
struct acpi_ac *ac;
int result;
@@ -214,9 +214,9 @@ static int acpi_ac_probe(struct platform_device *pdev)
if (!ac)
return -ENOMEM;
- ac->device = device;
- strcpy(acpi_device_name(device), ACPI_AC_DEVICE_NAME);
- strcpy(acpi_device_class(device), ACPI_AC_CLASS);
+ ac->device = adev;
+ strcpy(acpi_device_name(adev), ACPI_AC_DEVICE_NAME);
+ strcpy(acpi_device_class(adev), ACPI_AC_CLASS);
platform_set_drvdata(pdev, ac);
@@ -226,7 +226,7 @@ static int acpi_ac_probe(struct platform_device *pdev)
psy_cfg.drv_data = ac;
- ac->charger_desc.name = acpi_device_bid(device);
+ ac->charger_desc.name = acpi_device_bid(adev);
ac->charger_desc.type = POWER_SUPPLY_TYPE_MAINS;
ac->charger_desc.properties = ac_props;
ac->charger_desc.num_properties = ARRAY_SIZE(ac_props);
@@ -238,13 +238,13 @@ static int acpi_ac_probe(struct platform_device *pdev)
goto err_release_ac;
}
- pr_info("%s [%s] (%s-line)\n", acpi_device_name(device),
- acpi_device_bid(device), str_on_off(ac->state));
+ pr_info("%s [%s] (%s-line)\n", acpi_device_name(adev),
+ acpi_device_bid(adev), str_on_off(ac->state));
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
- result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY,
+ result = acpi_dev_install_notify_handler(adev, ACPI_ALL_NOTIFY,
acpi_ac_notify, >dev);
if (result)
goto err_unregister;
--
2.41.0
[PATCH v3 3/6] ACPI: AC: Replace acpi_driver with platform_driver
AC driver uses struct acpi_driver incorrectly to register itself. This
is wrong as the instances of the ACPI devices are not meant to
be literal devices, they're supposed to describe ACPI entry of a
particular device.
Use platform_driver instead of acpi_driver. In relevant places call
platform devices instances pdev to make a distinction with ACPI
devices instances.
Drop unnecessary casts from acpi_bus_generate_netlink_event() and
acpi_notifier_call_chain().
Add a blank line to distinguish pdev API vs local ACPI notify function.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 61 +--
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index f809f6889b4a..1dd4919be7ac 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -33,8 +33,9 @@ MODULE_AUTHOR("Paul Diefenbaugh");
MODULE_DESCRIPTION("ACPI AC Adapter Driver");
MODULE_LICENSE("GPL");
-static int acpi_ac_add(struct acpi_device *device);
-static void acpi_ac_remove(struct acpi_device *device);
+static int acpi_ac_probe(struct platform_device *pdev);
+static void acpi_ac_remove(struct platform_device *pdev);
+
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
static const struct acpi_device_id ac_device_ids[] = {
@@ -51,17 +52,6 @@ static SIMPLE_DEV_PM_OPS(acpi_ac_pm, NULL, acpi_ac_resume);
static int ac_sleep_before_get_state_ms;
static int ac_only;
-static struct acpi_driver acpi_ac_driver = {
- .name = "ac",
- .class = ACPI_AC_CLASS,
- .ids = ac_device_ids,
- .ops = {
- .add = acpi_ac_add,
- .remove = acpi_ac_remove,
- },
- .drv.pm = _ac_pm,
-};
-
struct acpi_ac {
struct power_supply *charger;
struct power_supply_desc charger_desc;
@@ -129,8 +119,9 @@ static enum power_supply_property ac_props[] = {
/* Driver Model */
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_device *device = data;
- struct acpi_ac *ac = acpi_driver_data(device);
+ struct device *dev = data;
+ struct acpi_ac *ac = dev_get_drvdata(dev);
+ struct acpi_device *device = ACPI_COMPANION(dev);
switch (event) {
default:
@@ -152,9 +143,10 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
acpi_ac_get_state(ac);
acpi_bus_generate_netlink_event(device->pnp.device_class,
- dev_name(>dev), event,
- (u32) ac->state);
- acpi_notifier_call_chain(device, event, (u32) ac->state);
+ dev_name(dev),
+ event,
+ ac->state);
+ acpi_notifier_call_chain(device, event, ac->state);
kobject_uevent(>charger->dev.kobj, KOBJ_CHANGE);
}
}
@@ -211,8 +203,9 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
{},
};
-static int acpi_ac_add(struct acpi_device *device)
+static int acpi_ac_probe(struct platform_device *pdev)
{
+ struct acpi_device *device = ACPI_COMPANION(>dev);
struct power_supply_config psy_cfg = {};
struct acpi_ac *ac;
int result;
@@ -224,7 +217,8 @@ static int acpi_ac_add(struct acpi_device *device)
ac->device = device;
strcpy(acpi_device_name(device), ACPI_AC_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_AC_CLASS);
- device->driver_data = ac;
+
+ platform_set_drvdata(pdev, ac);
result = acpi_ac_get_state(ac);
if (result)
@@ -237,7 +231,7 @@ static int acpi_ac_add(struct acpi_device *device)
ac->charger_desc.properties = ac_props;
ac->charger_desc.num_properties = ARRAY_SIZE(ac_props);
ac->charger_desc.get_property = get_ac_property;
- ac->charger = power_supply_register(>device->dev,
+ ac->charger = power_supply_register(>dev,
>charger_desc, _cfg);
if (IS_ERR(ac->charger)) {
result = PTR_ERR(ac->charger);
@@ -251,7 +245,7 @@ static int acpi_ac_add(struct acpi_device *device)
register_acpi_notifier(>battery_nb);
result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY,
-acpi_ac_notify, device);
+acpi_ac_notify, >dev);
if (result)
goto err_unregister;
@@ -269,7 +263,7 @@ static int acpi_ac_add(struct acpi_device *device)
#ifdef CONFIG_PM_SLEEP
static int acpi_ac_resume(struct device *dev)
{
- struct acpi_ac *ac = acpi_dri
[PATCH v3 1/6] ACPI: AC: Remove unnecessary checks
Remove unnecessary checks for NULL for variables that can't be NULL at
the point they're checked for it. Defensive programming is discouraged
in the kernel.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 27 ---
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index aac3e561790c..83d45c681121 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -131,9 +131,6 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
struct acpi_device *device = data;
struct acpi_ac *ac = acpi_driver_data(device);
- if (!ac)
- return;
-
switch (event) {
default:
acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
@@ -216,12 +213,8 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
static int acpi_ac_add(struct acpi_device *device)
{
struct power_supply_config psy_cfg = {};
- int result = 0;
- struct acpi_ac *ac = NULL;
-
-
- if (!device)
- return -EINVAL;
+ struct acpi_ac *ac;
+ int result;
ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
if (!ac)
@@ -275,16 +268,9 @@ static int acpi_ac_add(struct acpi_device *device)
#ifdef CONFIG_PM_SLEEP
static int acpi_ac_resume(struct device *dev)
{
- struct acpi_ac *ac;
+ struct acpi_ac *ac = acpi_driver_data(to_acpi_device(dev));
unsigned int old_state;
- if (!dev)
- return -EINVAL;
-
- ac = acpi_driver_data(to_acpi_device(dev));
- if (!ac)
- return -EINVAL;
-
old_state = ac->state;
if (acpi_ac_get_state(ac))
return 0;
@@ -299,12 +285,7 @@ static int acpi_ac_resume(struct device *dev)
static void acpi_ac_remove(struct acpi_device *device)
{
- struct acpi_ac *ac = NULL;
-
- if (!device || !acpi_driver_data(device))
- return;
-
- ac = acpi_driver_data(device);
+ struct acpi_ac *ac = acpi_driver_data(device);
acpi_dev_remove_notify_handler(device, ACPI_ALL_NOTIFY,
acpi_ac_notify);
--
2.41.0
[PATCH v3 2/6] ACPI: AC: Use string_choices API instead of ternary operator
Use modern string_choices API instead of manually determining the
output using ternary operator.
Suggested-by: Andy Shevchenko
Reviewed-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 83d45c681121..f809f6889b4a 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
@@ -243,8 +244,8 @@ static int acpi_ac_add(struct acpi_device *device)
goto err_release_ac;
}
- pr_info("%s [%s] (%s)\n", acpi_device_name(device),
- acpi_device_bid(device), ac->state ? "on-line" : "off-line");
+ pr_info("%s [%s] (%s-line)\n", acpi_device_name(device),
+ acpi_device_bid(device), str_on_off(ac->state));
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
--
2.41.0
[PATCH v3 0/6] Replace acpi_driver with platform_driver
Currently there is a number of drivers that somewhat incorrectly register themselves as acpi_driver, while they should be registering as platform_drivers. acpi_device was never meant to represent actual device, it only represents an entry in the ACPI table and ACPI driver should be treated as a driver for the ACPI entry of the particular device, not the device itself. Drivers of the devices itself should register as platform_drivers. Replace acpi_driver with platform_driver for all relevant drivers in drivers/acpi directory. This is just the first part of the change, as many acpi_drivers are present in many files outside of drivers/acpi directory. Additionally during internal review we've decided that it's best to send the relevant patches sequentially, in order not to overwhelm the reviewers. So I'm starting with NFIT and AC drivers. This change is possible thanks to recent .notify callback removal in drivers/acpi [1]. So flow for the remaining acpi_drivers will look like this: 1) Remove .notify callback with separate patchset for drivers/platform and other outstanding places like drivers/hwmon, drivers/iio etc. 2) Replace acpi_driver with platform_driver, as aside for .notify callback they're mostly functionally compatible. Most of the patches in this series could be considered independent, and could be merged separately, with a notable exception of the very first patch in this series that changes notify installer wrappers to be more generic. I decided it would be best not to force the user of this wrappers to pass any specific structure, like acpi_device or platform_device. It makes sense as it is very often the case that drivers declare their own private structure which gets allocated during the .probe() callback, and become the heart of the driver. In that case it makes much more sense to pass this structure to notify handler. Additionally some drivers, like acpi_video that already have custom notify handlers do that already. Link: https://lore.kernel.org/linux-acpi/20230703080252.2899090-1-michal.wilczyn...@intel.com/ [1] v3: - in AC transformation patch replaced struct device* with struct acpi_device*, as suggested. v2: - dropped first, second and last commit. Last commit was deemed pointless, first and second are already merged. - rebased on top of modified first commit (changes in the wrappers) Michal Wilczynski (6): ACPI: AC: Remove unnecessary checks ACPI: AC: Use string_choices API instead of ternary operator ACPI: AC: Replace acpi_driver with platform_driver ACPI: AC: Rename ACPI device from device to adev ACPI: NFIT: Replace acpi_driver with platform_driver ACPI: NFIT: Remove redundant call to to_acpi_dev() drivers/acpi/ac.c| 103 +-- drivers/acpi/nfit/core.c | 38 +++ 2 files changed, 64 insertions(+), 77 deletions(-) -- 2.41.0
[PATCH v2 6/6] ACPI: NFIT: Remove redundant call to to_acpi_dev()
In acpi_nfit_ctl() ACPI handle is extracted using to_acpi_dev()
function and accessing handle field in ACPI device. After transformation
from ACPI driver to platform driver this is not optimal anymore. To get
a handle it's enough to just use ACPI_HANDLE() macro to extract the
handle.
Suggested-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index fb0bc16fa186..3d254b2cf2e7 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -475,8 +475,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
guid = to_nfit_uuid(nfit_mem->family);
handle = adev->handle;
} else {
- struct acpi_device *adev = to_acpi_dev(acpi_desc);
-
cmd_name = nvdimm_bus_cmd_name(cmd);
cmd_mask = nd_desc->cmd_mask;
if (cmd == ND_CMD_CALL && call_pkg->nd_family) {
@@ -493,7 +491,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
guid = to_nfit_uuid(NFIT_DEV_BUS);
}
desc = nd_cmd_bus_desc(cmd);
- handle = adev->handle;
+ handle = ACPI_HANDLE(dev);
dimm_name = "bus";
}
--
2.41.0
[PATCH v2 4/6] ACPI: AC: Rename ACPI device from device to adev
Since transformation from ACPI driver to platform driver there are two
devices on which the driver operates - ACPI device and platform device.
For the sake of reader this calls for the distinction in their naming,
to avoid confusion. Rename device to adev, as corresponding
platform device is called pdev.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 298defeb5301..bb02e7f5d65a 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -120,7 +120,7 @@ static enum power_supply_property ac_props[] = {
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
struct acpi_ac *ac = data;
- struct acpi_device *device = ACPI_COMPANION(ac->dev);
+ struct acpi_device *adev = ACPI_COMPANION(ac->dev);
switch (event) {
default:
@@ -141,11 +141,11 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
msleep(ac_sleep_before_get_state_ms);
acpi_ac_get_state(ac);
- acpi_bus_generate_netlink_event(device->pnp.device_class,
+ acpi_bus_generate_netlink_event(adev->pnp.device_class,
dev_name(ac->dev),
event,
ac->state);
- acpi_notifier_call_chain(device, event, ac->state);
+ acpi_notifier_call_chain(adev, event, ac->state);
kobject_uevent(>charger->dev.kobj, KOBJ_CHANGE);
}
}
@@ -204,7 +204,7 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
static int acpi_ac_probe(struct platform_device *pdev)
{
- struct acpi_device *device = ACPI_COMPANION(>dev);
+ struct acpi_device *adev = ACPI_COMPANION(>dev);
struct power_supply_config psy_cfg = {};
struct acpi_ac *ac;
int result;
@@ -214,8 +214,8 @@ static int acpi_ac_probe(struct platform_device *pdev)
return -ENOMEM;
ac->dev = >dev;
- strcpy(acpi_device_name(device), ACPI_AC_DEVICE_NAME);
- strcpy(acpi_device_class(device), ACPI_AC_CLASS);
+ strcpy(acpi_device_name(adev), ACPI_AC_DEVICE_NAME);
+ strcpy(acpi_device_class(adev), ACPI_AC_CLASS);
platform_set_drvdata(pdev, ac);
@@ -225,7 +225,7 @@ static int acpi_ac_probe(struct platform_device *pdev)
psy_cfg.drv_data = ac;
- ac->charger_desc.name = acpi_device_bid(device);
+ ac->charger_desc.name = acpi_device_bid(adev);
ac->charger_desc.type = POWER_SUPPLY_TYPE_MAINS;
ac->charger_desc.properties = ac_props;
ac->charger_desc.num_properties = ARRAY_SIZE(ac_props);
@@ -237,13 +237,13 @@ static int acpi_ac_probe(struct platform_device *pdev)
goto err_release_ac;
}
- pr_info("%s [%s] (%s-line)\n", acpi_device_name(device),
- acpi_device_bid(device), str_on_off(ac->state));
+ pr_info("%s [%s] (%s-line)\n", acpi_device_name(adev),
+ acpi_device_bid(adev), str_on_off(ac->state));
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
- result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY,
+ result = acpi_dev_install_notify_handler(adev, ACPI_ALL_NOTIFY,
acpi_ac_notify, ac);
if (result)
goto err_unregister;
--
2.41.0
[PATCH v2 5/6] ACPI: NFIT: Replace acpi_driver with platform_driver
NFIT driver uses struct acpi_driver incorrectly to register itself.
This is wrong as the instances of the ACPI devices are not meant
to be literal devices, they're supposed to describe ACPI entry of a
particular device.
Use platform_driver instead of acpi_driver. In relevant places call
platform devices instances pdev to make a distinction with ACPI
devices instances.
NFIT driver uses devm_*() family of functions extensively. This change
has no impact on correct functioning of the whole devm_*() family of
functions, since the lifecycle of the device stays the same. It is still
being created during the enumeration, and destroyed on platform device
removal.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 942b84d94078..fb0bc16fa186 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include "intel.h"
@@ -98,7 +99,7 @@ static struct acpi_device *to_acpi_dev(struct acpi_nfit_desc
*acpi_desc)
|| strcmp(nd_desc->provider_name, "ACPI.NFIT") != 0)
return NULL;
- return to_acpi_device(acpi_desc->dev);
+ return ACPI_COMPANION(acpi_desc->dev);
}
static int xlat_bus_status(void *buf, unsigned int cmd, u32 status)
@@ -3284,11 +3285,11 @@ static void acpi_nfit_put_table(void *table)
static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_device *adev = data;
+ struct device *dev = data;
- device_lock(>dev);
- __acpi_nfit_notify(>dev, handle, event);
- device_unlock(>dev);
+ device_lock(dev);
+ __acpi_nfit_notify(dev, handle, event);
+ device_unlock(dev);
}
static void acpi_nfit_remove_notify_handler(void *data)
@@ -3329,11 +3330,12 @@ void acpi_nfit_shutdown(void *data)
}
EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
-static int acpi_nfit_add(struct acpi_device *adev)
+static int acpi_nfit_probe(struct platform_device *pdev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_nfit_desc *acpi_desc;
- struct device *dev = >dev;
+ struct device *dev = >dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct acpi_table_header *tbl;
acpi_status status = AE_OK;
acpi_size sz;
@@ -3360,7 +3362,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
acpi_desc = devm_kzalloc(dev, sizeof(*acpi_desc), GFP_KERNEL);
if (!acpi_desc)
return -ENOMEM;
- acpi_nfit_desc_init(acpi_desc, >dev);
+ acpi_nfit_desc_init(acpi_desc, dev);
/* Save the acpi header for exporting the revision via sysfs */
acpi_desc->acpi_header = *tbl;
@@ -3391,7 +3393,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
return rc;
rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
-acpi_nfit_notify, adev);
+acpi_nfit_notify, dev);
if (rc)
return rc;
@@ -3475,11 +3477,11 @@ static const struct acpi_device_id acpi_nfit_ids[] = {
};
MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
-static struct acpi_driver acpi_nfit_driver = {
- .name = KBUILD_MODNAME,
- .ids = acpi_nfit_ids,
- .ops = {
- .add = acpi_nfit_add,
+static struct platform_driver acpi_nfit_driver = {
+ .probe = acpi_nfit_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .acpi_match_table = acpi_nfit_ids,
},
};
@@ -3517,7 +3519,7 @@ static __init int nfit_init(void)
return -ENOMEM;
nfit_mce_register();
- ret = acpi_bus_register_driver(_nfit_driver);
+ ret = platform_driver_register(_nfit_driver);
if (ret) {
nfit_mce_unregister();
destroy_workqueue(nfit_wq);
@@ -3530,7 +3532,7 @@ static __init int nfit_init(void)
static __exit void nfit_exit(void)
{
nfit_mce_unregister();
- acpi_bus_unregister_driver(_nfit_driver);
+ platform_driver_unregister(_nfit_driver);
destroy_workqueue(nfit_wq);
WARN_ON(!list_empty(_descs));
}
--
2.41.0
[PATCH v2 3/6] ACPI: AC: Replace acpi_driver with platform_driver
AC driver uses struct acpi_driver incorrectly to register itself. This
is wrong as the instances of the ACPI devices are not meant to
be literal devices, they're supposed to describe ACPI entry of a
particular device.
Use platform_driver instead of acpi_driver. In relevant places call
platform devices instances pdev to make a distinction with ACPI
devices instances.
Drop unnecessary casts from acpi_bus_generate_netlink_event() and
acpi_notifier_call_chain().
Add a blank line to distinguish pdev API vs local ACPI notify function.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 70 +--
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index f809f6889b4a..298defeb5301 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -33,8 +33,9 @@ MODULE_AUTHOR("Paul Diefenbaugh");
MODULE_DESCRIPTION("ACPI AC Adapter Driver");
MODULE_LICENSE("GPL");
-static int acpi_ac_add(struct acpi_device *device);
-static void acpi_ac_remove(struct acpi_device *device);
+static int acpi_ac_probe(struct platform_device *pdev);
+static void acpi_ac_remove(struct platform_device *pdev);
+
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
static const struct acpi_device_id ac_device_ids[] = {
@@ -51,21 +52,10 @@ static SIMPLE_DEV_PM_OPS(acpi_ac_pm, NULL, acpi_ac_resume);
static int ac_sleep_before_get_state_ms;
static int ac_only;
-static struct acpi_driver acpi_ac_driver = {
- .name = "ac",
- .class = ACPI_AC_CLASS,
- .ids = ac_device_ids,
- .ops = {
- .add = acpi_ac_add,
- .remove = acpi_ac_remove,
- },
- .drv.pm = _ac_pm,
-};
-
struct acpi_ac {
struct power_supply *charger;
struct power_supply_desc charger_desc;
- struct acpi_device *device;
+ struct device *dev;
unsigned long long state;
struct notifier_block battery_nb;
};
@@ -85,10 +75,10 @@ static int acpi_ac_get_state(struct acpi_ac *ac)
return 0;
}
- status = acpi_evaluate_integer(ac->device->handle, "_PSR", NULL,
+ status = acpi_evaluate_integer(ACPI_HANDLE(ac->dev), "_PSR", NULL,
>state);
if (ACPI_FAILURE(status)) {
- acpi_handle_info(ac->device->handle,
+ acpi_handle_info(ACPI_HANDLE(ac->dev),
"Error reading AC Adapter state: %s\n",
acpi_format_exception(status));
ac->state = ACPI_AC_STATUS_UNKNOWN;
@@ -129,12 +119,12 @@ static enum power_supply_property ac_props[] = {
/* Driver Model */
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_device *device = data;
- struct acpi_ac *ac = acpi_driver_data(device);
+ struct acpi_ac *ac = data;
+ struct acpi_device *device = ACPI_COMPANION(ac->dev);
switch (event) {
default:
- acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
+ acpi_handle_debug(ACPI_HANDLE(ac->dev), "Unsupported event
[0x%x]\n",
event);
fallthrough;
case ACPI_AC_NOTIFY_STATUS:
@@ -152,9 +142,10 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
acpi_ac_get_state(ac);
acpi_bus_generate_netlink_event(device->pnp.device_class,
- dev_name(>dev), event,
- (u32) ac->state);
- acpi_notifier_call_chain(device, event, (u32) ac->state);
+ dev_name(ac->dev),
+ event,
+ ac->state);
+ acpi_notifier_call_chain(device, event, ac->state);
kobject_uevent(>charger->dev.kobj, KOBJ_CHANGE);
}
}
@@ -211,8 +202,9 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
{},
};
-static int acpi_ac_add(struct acpi_device *device)
+static int acpi_ac_probe(struct platform_device *pdev)
{
+ struct acpi_device *device = ACPI_COMPANION(>dev);
struct power_supply_config psy_cfg = {};
struct acpi_ac *ac;
int result;
@@ -221,10 +213,11 @@ static int acpi_ac_add(struct acpi_device *device)
if (!ac)
return -ENOMEM;
- ac->device = device;
+ ac->dev = >dev;
strcpy(acpi_device_name(device), ACPI_AC_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_AC_CLASS);
- device->driver_data = ac;
+
+ platform_s
[PATCH v2 2/6] ACPI: AC: Use string_choices API instead of ternary operator
Use modern string_choices API instead of manually determining the
output using ternary operator.
Suggested-by: Andy Shevchenko
Reviewed-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 83d45c681121..f809f6889b4a 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
@@ -243,8 +244,8 @@ static int acpi_ac_add(struct acpi_device *device)
goto err_release_ac;
}
- pr_info("%s [%s] (%s)\n", acpi_device_name(device),
- acpi_device_bid(device), ac->state ? "on-line" : "off-line");
+ pr_info("%s [%s] (%s-line)\n", acpi_device_name(device),
+ acpi_device_bid(device), str_on_off(ac->state));
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
--
2.41.0
[PATCH v2 1/6] ACPI: AC: Remove unnecessary checks
Remove unnecessary checks for NULL for variables that can't be NULL at
the point they're checked for it. Defensive programming is discouraged
in the kernel.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 27 ---
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index aac3e561790c..83d45c681121 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -131,9 +131,6 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
struct acpi_device *device = data;
struct acpi_ac *ac = acpi_driver_data(device);
- if (!ac)
- return;
-
switch (event) {
default:
acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
@@ -216,12 +213,8 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
static int acpi_ac_add(struct acpi_device *device)
{
struct power_supply_config psy_cfg = {};
- int result = 0;
- struct acpi_ac *ac = NULL;
-
-
- if (!device)
- return -EINVAL;
+ struct acpi_ac *ac;
+ int result;
ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
if (!ac)
@@ -275,16 +268,9 @@ static int acpi_ac_add(struct acpi_device *device)
#ifdef CONFIG_PM_SLEEP
static int acpi_ac_resume(struct device *dev)
{
- struct acpi_ac *ac;
+ struct acpi_ac *ac = acpi_driver_data(to_acpi_device(dev));
unsigned int old_state;
- if (!dev)
- return -EINVAL;
-
- ac = acpi_driver_data(to_acpi_device(dev));
- if (!ac)
- return -EINVAL;
-
old_state = ac->state;
if (acpi_ac_get_state(ac))
return 0;
@@ -299,12 +285,7 @@ static int acpi_ac_resume(struct device *dev)
static void acpi_ac_remove(struct acpi_device *device)
{
- struct acpi_ac *ac = NULL;
-
- if (!device || !acpi_driver_data(device))
- return;
-
- ac = acpi_driver_data(device);
+ struct acpi_ac *ac = acpi_driver_data(device);
acpi_dev_remove_notify_handler(device, ACPI_ALL_NOTIFY,
acpi_ac_notify);
--
2.41.0
[PATCH v2 0/6] Replace acpi_driver with platform_driver
Currently there is a number of drivers that somewhat incorrectly register themselves as acpi_driver, while they should be registering as platform_drivers. acpi_device was never meant to represent actual device, it only represents an entry in the ACPI table and ACPI driver should be treated as a driver for the ACPI entry of the particular device, not the device itself. Drivers of the devices itself should register as platform_drivers. Replace acpi_driver with platform_driver for all relevant drivers in drivers/acpi directory. This is just the first part of the change, as many acpi_drivers are present in many files outside of drivers/acpi directory. Additionally during internal review we've decided that it's best to send the relevant patches sequentially, in order not to overwhelm the reviewers. So I'm starting with NFIT and AC drivers. This change is possible thanks to recent .notify callback removal in drivers/acpi [1]. So flow for the remaining acpi_drivers will look like this: 1) Remove .notify callback with separate patchset for drivers/platform and other outstanding places like drivers/hwmon, drivers/iio etc. 2) Replace acpi_driver with platform_driver, as aside for .notify callback they're mostly functionally compatible. Most of the patches in this series could be considered independent, and could be merged separately, with a notable exception of the very first patch in this series that changes notify installer wrappers to be more generic. I decided it would be best not to force the user of this wrappers to pass any specific structure, like acpi_device or platform_device. It makes sense as it is very often the case that drivers declare their own private structure which gets allocated during the .probe() callback, and become the heart of the driver. In that case it makes much more sense to pass this structure to notify handler. Additionally some drivers, like acpi_video that already have custom notify handlers do that already. Link: https://lore.kernel.org/linux-acpi/20230703080252.2899090-1-michal.wilczyn...@intel.com/ [1] v2: - dropped first, second and last commit. Last commit was deemed pointless, first and second are already merged. - rebased on top of modified first commit (changes in the wrappers) Michal Wilczynski (6): ACPI: AC: Remove unnecessary checks ACPI: AC: Use string_choices API instead of ternary operator ACPI: AC: Replace acpi_driver with platform_driver ACPI: AC: Rename ACPI device from device to adev ACPI: NFIT: Replace acpi_driver with platform_driver ACPI: NFIT: Remove redundant call to to_acpi_dev() drivers/acpi/ac.c| 108 +-- drivers/acpi/nfit/core.c | 38 +++--- 2 files changed, 66 insertions(+), 80 deletions(-) -- 2.41.0
[PATCH v1 1/2] ACPI: NFIT: Fix memory leak, and local use of devm_*()
devm_*() family of functions purpose is managing memory attached to a
device. So in general it should only be used for allocations that should
last for the whole lifecycle of the device. This is not the case for
acpi_nfit_init_interleave_set(). There are two allocations that are only
used locally in this function. What's more - if the function exits on
error path memory is never freed. It's still attached to dev and would
be freed on device detach, so this leak could be called a 'local leak'.
Fix this by switching from devm_kcalloc() to kcalloc(), and adding
proper rollback.
Fixes: eaf961536e16 ("libnvdimm, nfit: add interleave-set state-tracking
infrastructure")
Reported-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index f0e6738ae3c9..78f0f855c4de 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2262,6 +2262,7 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
u16 nr = ndr_desc->num_mappings;
struct nfit_set_info2 *info2;
struct nfit_set_info *info;
+ int err = 0;
int i;
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
@@ -2269,13 +2270,15 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
return -ENOMEM;
import_guid(_set->type_guid, spa->range_guid);
- info = devm_kcalloc(dev, nr, sizeof(*info), GFP_KERNEL);
+ info = kcalloc(nr, sizeof(*info), GFP_KERNEL);
if (!info)
return -ENOMEM;
- info2 = devm_kcalloc(dev, nr, sizeof(*info2), GFP_KERNEL);
- if (!info2)
- return -ENOMEM;
+ info2 = kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ if (!info2) {
+ err = -ENOMEM;
+ goto free_info;
+ }
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = _desc->mapping[i];
@@ -2289,7 +2292,8 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
if (!memdev || !nfit_mem->dcr) {
dev_err(dev, "%s: failed to find DCR\n", __func__);
- return -ENODEV;
+ err = -ENODEV;
+ goto free_info2;
}
map->region_offset = memdev->region_offset;
@@ -2337,10 +2341,13 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
}
ndr_desc->nd_set = nd_set;
- devm_kfree(dev, info);
- devm_kfree(dev, info2);
- return 0;
+free_info2:
+ kfree(info2);
+free_info:
+ kfree(info);
+
+ return err;
}
static int ars_get_cap(struct acpi_nfit_desc *acpi_desc,
--
2.41.0
[PATCH v1 2/2] ACPI: NFIT: Use modern scope based rollback
Change rollback in acpi_nfit_init_interleave_set() to use modern scope
based attribute __free(). This is similar to C++ RAII and is a preferred
way for handling local memory allocations.
Suggested-by: Dave Jiang
Suggested-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 32 ++--
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 78f0f855c4de..079bd663495f 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2257,29 +2257,23 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
struct nd_region_desc *ndr_desc,
struct acpi_nfit_system_address *spa)
{
+ u16 nr = ndr_desc->num_mappings;
+ struct nfit_set_info2 *info2 __free(kfree) =
+ kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ struct nfit_set_info *info __free(kfree) =
+ kcalloc(nr, sizeof(*info), GFP_KERNEL);
struct device *dev = acpi_desc->dev;
struct nd_interleave_set *nd_set;
- u16 nr = ndr_desc->num_mappings;
- struct nfit_set_info2 *info2;
- struct nfit_set_info *info;
- int err = 0;
int i;
+ if (!info || !info2)
+ return -ENOMEM;
+
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
if (!nd_set)
return -ENOMEM;
import_guid(_set->type_guid, spa->range_guid);
- info = kcalloc(nr, sizeof(*info), GFP_KERNEL);
- if (!info)
- return -ENOMEM;
-
- info2 = kcalloc(nr, sizeof(*info2), GFP_KERNEL);
- if (!info2) {
- err = -ENOMEM;
- goto free_info;
- }
-
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = _desc->mapping[i];
struct nvdimm *nvdimm = mapping->nvdimm;
@@ -2292,8 +2286,7 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
if (!memdev || !nfit_mem->dcr) {
dev_err(dev, "%s: failed to find DCR\n", __func__);
- err = -ENODEV;
- goto free_info2;
+ return -ENODEV;
}
map->region_offset = memdev->region_offset;
@@ -2342,12 +2335,7 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
ndr_desc->nd_set = nd_set;
-free_info2:
- kfree(info2);
-free_info:
- kfree(info);
-
- return err;
+ return 0;
}
static int ars_get_cap(struct acpi_nfit_desc *acpi_desc,
--
2.41.0
[PATCH v1 0/2] Fix memory leak and move to modern scope based rollback
In acpi_nfit_init_interleave_set() there is a memory leak + improper use of devm_*() family of functions for local memory allocations. This patch series provides two commits - one is meant as a bug fix, and could potentially be backported, and the other one improves old style rollback with scope based, similar to C++ RAII [1]. Link: https://lwn.net/Articles/934679/ [1] Michal Wilczynski (2): ACPI: NFIT: Fix memory leak, and local use of devm_*() ACPI: NFIT: Use modern scope based rollback drivers/acpi/nfit/core.c | 21 - 1 file changed, 8 insertions(+), 13 deletions(-) -- 2.41.0
[PATCH v1 9/9] ACPI: NFIT: Don't use KBUILD_MODNAME for driver name
Driver name is part of the ABI, so it should be hard-coded, as ABI
should be always kept backward compatible. Prevent ABI from changing
accidentally in case KBUILD_MODNAME change.
Suggested-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index f09530d2520a..987eb5567036 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3478,7 +3478,7 @@ MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
static struct platform_driver acpi_nfit_driver = {
.probe = acpi_nfit_probe,
.driver = {
- .name = KBUILD_MODNAME,
+ .name = "nfit",
.acpi_match_table = acpi_nfit_ids,
},
};
--
2.41.0
[PATCH v1 8/9] ACPI: NFIT: Remove redundant call to to_acpi_dev()
In acpi_nfit_ctl() ACPI handle is extracted using to_acpi_dev()
function and accessing handle field in ACPI device. After transformation
from ACPI driver to platform driver this is not optimal anymore. To get
a handle it's enough to just use ACPI_HANDLE() macro to extract the
handle.
Suggested-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 2e50b1334a69..f09530d2520a 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -475,8 +475,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
guid = to_nfit_uuid(nfit_mem->family);
handle = adev->handle;
} else {
- struct acpi_device *adev = to_acpi_dev(acpi_desc);
-
cmd_name = nvdimm_bus_cmd_name(cmd);
cmd_mask = nd_desc->cmd_mask;
if (cmd == ND_CMD_CALL && call_pkg->nd_family) {
@@ -493,7 +491,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
guid = to_nfit_uuid(NFIT_DEV_BUS);
}
desc = nd_cmd_bus_desc(cmd);
- handle = adev->handle;
+ handle = ACPI_HANDLE(dev);
dimm_name = "bus";
}
--
2.41.0
[PATCH v1 7/9] ACPI: NFIT: Replace acpi_driver with platform_driver
NFIT driver uses struct acpi_driver incorrectly to register itself.
This is wrong as the instances of the ACPI devices are not meant
to be literal devices, they're supposed to describe ACPI entry of a
particular device.
Use platform_driver instead of acpi_driver. In relevant places call
platform devices instances pdev to make a distinction with ACPI
devices instances.
NFIT driver uses devm_*() family of functions extensively. This change
has no impact on correct functioning of the whole devm_*() family of
functions, since the lifecycle of the device stays the same. It is still
being created during the enumeration, and destroyed on platform device
removal.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 821870f57862..2e50b1334a69 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include "intel.h"
@@ -98,7 +99,7 @@ static struct acpi_device *to_acpi_dev(struct acpi_nfit_desc
*acpi_desc)
|| strcmp(nd_desc->provider_name, "ACPI.NFIT") != 0)
return NULL;
- return to_acpi_device(acpi_desc->dev);
+ return ACPI_COMPANION(acpi_desc->dev);
}
static int xlat_bus_status(void *buf, unsigned int cmd, u32 status)
@@ -3284,11 +3285,11 @@ static void acpi_nfit_put_table(void *table)
static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_device *adev = data;
+ struct device *dev = data;
- device_lock(>dev);
- __acpi_nfit_notify(>dev, handle, event);
- device_unlock(>dev);
+ device_lock(dev);
+ __acpi_nfit_notify(dev, handle, event);
+ device_unlock(dev);
}
static void acpi_nfit_remove_notify_handler(void *data)
@@ -3329,11 +3330,12 @@ void acpi_nfit_shutdown(void *data)
}
EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
-static int acpi_nfit_add(struct acpi_device *adev)
+static int acpi_nfit_probe(struct platform_device *pdev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_nfit_desc *acpi_desc;
- struct device *dev = >dev;
+ struct device *dev = >dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct acpi_table_header *tbl;
acpi_status status = AE_OK;
acpi_size sz;
@@ -3360,7 +3362,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
acpi_desc = devm_kzalloc(dev, sizeof(*acpi_desc), GFP_KERNEL);
if (!acpi_desc)
return -ENOMEM;
- acpi_nfit_desc_init(acpi_desc, >dev);
+ acpi_nfit_desc_init(acpi_desc, dev);
/* Save the acpi header for exporting the revision via sysfs */
acpi_desc->acpi_header = *tbl;
@@ -3391,7 +3393,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
return rc;
rc = acpi_dev_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
-acpi_nfit_notify, adev);
+acpi_nfit_notify, dev);
if (rc)
return rc;
@@ -3475,11 +3477,11 @@ static const struct acpi_device_id acpi_nfit_ids[] = {
};
MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
-static struct acpi_driver acpi_nfit_driver = {
- .name = KBUILD_MODNAME,
- .ids = acpi_nfit_ids,
- .ops = {
- .add = acpi_nfit_add,
+static struct platform_driver acpi_nfit_driver = {
+ .probe = acpi_nfit_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .acpi_match_table = acpi_nfit_ids,
},
};
@@ -3517,7 +3519,7 @@ static __init int nfit_init(void)
return -ENOMEM;
nfit_mce_register();
- ret = acpi_bus_register_driver(_nfit_driver);
+ ret = platform_driver_register(_nfit_driver);
if (ret) {
nfit_mce_unregister();
destroy_workqueue(nfit_wq);
@@ -3530,7 +3532,7 @@ static __init int nfit_init(void)
static __exit void nfit_exit(void)
{
nfit_mce_unregister();
- acpi_bus_unregister_driver(_nfit_driver);
+ platform_driver_unregister(_nfit_driver);
destroy_workqueue(nfit_wq);
WARN_ON(!list_empty(_descs));
}
--
2.41.0
[PATCH v1 6/9] ACPI: AC: Rename ACPI device from device to adev
Since transformation from ACPI driver to platform driver there are two
devices on which the driver operates - ACPI device and platform device.
For the sake of reader this calls for the distinction in their naming,
to avoid confusion. Rename device to adev, as corresponding
platform device is called pdev.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 78e53d0fdece..270a6919ec12 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -120,7 +120,7 @@ static enum power_supply_property ac_props[] = {
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
struct acpi_ac *ac = data;
- struct acpi_device *device = ACPI_COMPANION(ac->dev);
+ struct acpi_device *adev = ACPI_COMPANION(ac->dev);
switch (event) {
default:
@@ -141,11 +141,11 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
msleep(ac_sleep_before_get_state_ms);
acpi_ac_get_state(ac);
- acpi_bus_generate_netlink_event(device->pnp.device_class,
+ acpi_bus_generate_netlink_event(adev->pnp.device_class,
dev_name(ac->dev),
event,
ac->state);
- acpi_notifier_call_chain(device, event, ac->state);
+ acpi_notifier_call_chain(adev, event, ac->state);
kobject_uevent(>charger->dev.kobj, KOBJ_CHANGE);
}
}
@@ -204,7 +204,7 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
static int acpi_ac_probe(struct platform_device *pdev)
{
- struct acpi_device *device = ACPI_COMPANION(>dev);
+ struct acpi_device *adev = ACPI_COMPANION(>dev);
struct power_supply_config psy_cfg = {};
struct acpi_ac *ac;
int result;
@@ -214,8 +214,8 @@ static int acpi_ac_probe(struct platform_device *pdev)
return -ENOMEM;
ac->dev = >dev;
- strcpy(acpi_device_name(device), ACPI_AC_DEVICE_NAME);
- strcpy(acpi_device_class(device), ACPI_AC_CLASS);
+ strcpy(acpi_device_name(adev), ACPI_AC_DEVICE_NAME);
+ strcpy(acpi_device_class(adev), ACPI_AC_CLASS);
platform_set_drvdata(pdev, ac);
@@ -225,7 +225,7 @@ static int acpi_ac_probe(struct platform_device *pdev)
psy_cfg.drv_data = ac;
- ac->charger_desc.name = acpi_device_bid(device);
+ ac->charger_desc.name = acpi_device_bid(adev);
ac->charger_desc.type = POWER_SUPPLY_TYPE_MAINS;
ac->charger_desc.properties = ac_props;
ac->charger_desc.num_properties = ARRAY_SIZE(ac_props);
@@ -237,13 +237,13 @@ static int acpi_ac_probe(struct platform_device *pdev)
goto err_release_ac;
}
- pr_info("%s [%s] (%s-line)\n", acpi_device_name(device),
- acpi_device_bid(device), str_on_off(ac->state));
+ pr_info("%s [%s] (%s-line)\n", acpi_device_name(adev),
+ acpi_device_bid(adev), str_on_off(ac->state));
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
- result = acpi_dev_install_notify_handler(device->handle,
ACPI_ALL_NOTIFY,
+ result = acpi_dev_install_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
acpi_ac_notify, ac);
if (result)
goto err_unregister;
--
2.41.0
[PATCH v1 5/9] ACPI: AC: Replace acpi_driver with platform_driver
AC driver uses struct acpi_driver incorrectly to register itself. This
is wrong as the instances of the ACPI devices are not meant to
be literal devices, they're supposed to describe ACPI entry of a
particular device.
Use platform_driver instead of acpi_driver. In relevant places call
platform devices instances pdev to make a distinction with ACPI
devices instances.
Drop unnecessary casts from acpi_bus_generate_netlink_event() and
acpi_notifier_call_chain().
Add a blank line to distinguish pdev API vs local ACPI notify function.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 70 +--
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index fd8b392c19f4..78e53d0fdece 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -33,8 +33,9 @@ MODULE_AUTHOR("Paul Diefenbaugh");
MODULE_DESCRIPTION("ACPI AC Adapter Driver");
MODULE_LICENSE("GPL");
-static int acpi_ac_add(struct acpi_device *device);
-static void acpi_ac_remove(struct acpi_device *device);
+static int acpi_ac_probe(struct platform_device *pdev);
+static void acpi_ac_remove(struct platform_device *pdev);
+
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
static const struct acpi_device_id ac_device_ids[] = {
@@ -51,21 +52,10 @@ static SIMPLE_DEV_PM_OPS(acpi_ac_pm, NULL, acpi_ac_resume);
static int ac_sleep_before_get_state_ms;
static int ac_only;
-static struct acpi_driver acpi_ac_driver = {
- .name = "ac",
- .class = ACPI_AC_CLASS,
- .ids = ac_device_ids,
- .ops = {
- .add = acpi_ac_add,
- .remove = acpi_ac_remove,
- },
- .drv.pm = _ac_pm,
-};
-
struct acpi_ac {
struct power_supply *charger;
struct power_supply_desc charger_desc;
- struct acpi_device *device;
+ struct device *dev;
unsigned long long state;
struct notifier_block battery_nb;
};
@@ -85,10 +75,10 @@ static int acpi_ac_get_state(struct acpi_ac *ac)
return 0;
}
- status = acpi_evaluate_integer(ac->device->handle, "_PSR", NULL,
+ status = acpi_evaluate_integer(ACPI_HANDLE(ac->dev), "_PSR", NULL,
>state);
if (ACPI_FAILURE(status)) {
- acpi_handle_info(ac->device->handle,
+ acpi_handle_info(ACPI_HANDLE(ac->dev),
"Error reading AC Adapter state: %s\n",
acpi_format_exception(status));
ac->state = ACPI_AC_STATUS_UNKNOWN;
@@ -129,12 +119,12 @@ static enum power_supply_property ac_props[] = {
/* Driver Model */
static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_device *device = data;
- struct acpi_ac *ac = acpi_driver_data(device);
+ struct acpi_ac *ac = data;
+ struct acpi_device *device = ACPI_COMPANION(ac->dev);
switch (event) {
default:
- acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
+ acpi_handle_debug(ACPI_HANDLE(ac->dev), "Unsupported event
[0x%x]\n",
event);
fallthrough;
case ACPI_AC_NOTIFY_STATUS:
@@ -152,9 +142,10 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
acpi_ac_get_state(ac);
acpi_bus_generate_netlink_event(device->pnp.device_class,
- dev_name(>dev), event,
- (u32) ac->state);
- acpi_notifier_call_chain(device, event, (u32) ac->state);
+ dev_name(ac->dev),
+ event,
+ ac->state);
+ acpi_notifier_call_chain(device, event, ac->state);
kobject_uevent(>charger->dev.kobj, KOBJ_CHANGE);
}
}
@@ -211,8 +202,9 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
{},
};
-static int acpi_ac_add(struct acpi_device *device)
+static int acpi_ac_probe(struct platform_device *pdev)
{
+ struct acpi_device *device = ACPI_COMPANION(>dev);
struct power_supply_config psy_cfg = {};
struct acpi_ac *ac;
int result;
@@ -221,10 +213,11 @@ static int acpi_ac_add(struct acpi_device *device)
if (!ac)
return -ENOMEM;
- ac->device = device;
+ ac->dev = >dev;
strcpy(acpi_device_name(device), ACPI_AC_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_AC_CLASS);
- device->driver_data = ac;
+
+ platform_s
[PATCH v1 4/9] ACPI: AC: Use string_choices API instead of ternary operator
Use modern string_choices API instead of manually determining the
output using ternary operator.
Suggested-by: Andy Shevchenko
Reviewed-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index dd04809a787c..fd8b392c19f4 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
@@ -243,8 +244,8 @@ static int acpi_ac_add(struct acpi_device *device)
goto err_release_ac;
}
- pr_info("%s [%s] (%s)\n", acpi_device_name(device),
- acpi_device_bid(device), ac->state ? "on-line" : "off-line");
+ pr_info("%s [%s] (%s-line)\n", acpi_device_name(device),
+ acpi_device_bid(device), str_on_off(ac->state));
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
--
2.41.0
[PATCH v1 1/9] ACPI: bus: Make notify wrappers more generic
acpi_dev_install_notify_handler() and acpi_dev_remove_notify_handler() are wrappers around ACPICA installers. They are meant to save some duplicated code from drivers. However as we're moving towards drivers operating on platform_device they become a bit inconvenient to use as inside the driver code we mostly want to use driver data of platform device instead of ACPI device. Make notify handlers installer wrappers more generic, while still saving some code that would be duplicated otherwise. Reviewed-by: Andy Shevchenko Signed-off-by: Michal Wilczynski --- Notes: So one solution could be to just replace acpi_device with platform_device as an argument in those functions. However I don't believe this is a correct solution, as it is very often the case that drivers declare their own private structures which gets allocated during the .probe() callback, and become the heart of the driver. When drivers do that it makes much more sense to just pass the private structure to the notify handler instead of forcing user to dance with the platform_device or acpi_device. drivers/acpi/ac.c | 6 +++--- drivers/acpi/acpi_video.c | 6 +++--- drivers/acpi/battery.c| 6 +++--- drivers/acpi/bus.c| 14 ++ drivers/acpi/hed.c| 6 +++--- drivers/acpi/nfit/core.c | 6 +++--- drivers/acpi/thermal.c| 6 +++--- include/acpi/acpi_bus.h | 9 - 8 files changed, 28 insertions(+), 31 deletions(-) diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c index 225dc6818751..0b245f9f7ec8 100644 --- a/drivers/acpi/ac.c +++ b/drivers/acpi/ac.c @@ -256,8 +256,8 @@ static int acpi_ac_add(struct acpi_device *device) ac->battery_nb.notifier_call = acpi_ac_battery_notify; register_acpi_notifier(>battery_nb); - result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY, -acpi_ac_notify); + result = acpi_dev_install_notify_handler(device->handle, ACPI_ALL_NOTIFY, +acpi_ac_notify, device); if (result) goto err_unregister; @@ -306,7 +306,7 @@ static void acpi_ac_remove(struct acpi_device *device) ac = acpi_driver_data(device); - acpi_dev_remove_notify_handler(device, ACPI_ALL_NOTIFY, + acpi_dev_remove_notify_handler(device->handle, ACPI_ALL_NOTIFY, acpi_ac_notify); power_supply_unregister(ac->charger); unregister_acpi_notifier(>battery_nb); diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index 948e31f7ce6e..025c17890127 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -2059,8 +2059,8 @@ static int acpi_video_bus_add(struct acpi_device *device) acpi_video_bus_add_notify_handler(video); - error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY, - acpi_video_bus_notify); + error = acpi_dev_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + acpi_video_bus_notify, device); if (error) goto err_remove; @@ -2092,7 +2092,7 @@ static void acpi_video_bus_remove(struct acpi_device *device) video = acpi_driver_data(device); - acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY, + acpi_dev_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, acpi_video_bus_notify); mutex_lock(_list_lock); diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 969bf81e8d54..45dae32a8646 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -1213,8 +1213,8 @@ static int acpi_battery_add(struct acpi_device *device) device_init_wakeup(>dev, 1); - result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY, -acpi_battery_notify); + result = acpi_dev_install_notify_handler(device->handle, ACPI_ALL_NOTIFY, +acpi_battery_notify, device); if (result) goto fail_pm; @@ -1241,7 +1241,7 @@ static void acpi_battery_remove(struct acpi_device *device) battery = acpi_driver_data(device); - acpi_dev_remove_notify_handler(device, ACPI_ALL_NOTIFY, + acpi_dev_remove_notify_handler(device->handle, ACPI_ALL_NOTIFY, acpi_battery_notify); device_init_wakeup(>dev, 0); diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index f41dda2d3493..479fe888d629 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -554,14 +554,13 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, acpi_os_wait_events_complete(); } -int acpi_dev_install_not
[PATCH v1 3/9] ACPI: AC: Remove unnecessary checks
Remove unnecessary checks for NULL for variables that can't be NULL at
the point they're checked for it. Defensive programming is discouraged
in the kernel.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 27 ---
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 0b245f9f7ec8..dd04809a787c 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -131,9 +131,6 @@ static void acpi_ac_notify(acpi_handle handle, u32 event,
void *data)
struct acpi_device *device = data;
struct acpi_ac *ac = acpi_driver_data(device);
- if (!ac)
- return;
-
switch (event) {
default:
acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",
@@ -216,12 +213,8 @@ static const struct dmi_system_id ac_dmi_table[]
__initconst = {
static int acpi_ac_add(struct acpi_device *device)
{
struct power_supply_config psy_cfg = {};
- int result = 0;
- struct acpi_ac *ac = NULL;
-
-
- if (!device)
- return -EINVAL;
+ struct acpi_ac *ac;
+ int result;
ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
if (!ac)
@@ -275,16 +268,9 @@ static int acpi_ac_add(struct acpi_device *device)
#ifdef CONFIG_PM_SLEEP
static int acpi_ac_resume(struct device *dev)
{
- struct acpi_ac *ac;
+ struct acpi_ac *ac = acpi_driver_data(to_acpi_device(dev));
unsigned int old_state;
- if (!dev)
- return -EINVAL;
-
- ac = acpi_driver_data(to_acpi_device(dev));
- if (!ac)
- return -EINVAL;
-
old_state = ac->state;
if (acpi_ac_get_state(ac))
return 0;
@@ -299,12 +285,7 @@ static int acpi_ac_resume(struct device *dev)
static void acpi_ac_remove(struct acpi_device *device)
{
- struct acpi_ac *ac = NULL;
-
- if (!device || !acpi_driver_data(device))
- return;
-
- ac = acpi_driver_data(device);
+ struct acpi_ac *ac = acpi_driver_data(device);
acpi_dev_remove_notify_handler(device->handle, ACPI_ALL_NOTIFY,
acpi_ac_notify);
--
2.41.0
[PATCH v1 2/9] docs: firmware-guide: ACPI: Clarify ACPI bus concepts
Some devices implement ACPI driver as a way to manage devices enumerated by the ACPI. This might be confusing as a preferred way to implement a driver for devices not connected to any bus is a platform driver, as stated in the documentation. Clarify relationships between ACPI device, platform device and ACPI entries. Suggested-by: Elena Reshetova Signed-off-by: Michal Wilczynski --- Documentation/firmware-guide/acpi/enumeration.rst | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/firmware-guide/acpi/enumeration.rst b/Documentation/firmware-guide/acpi/enumeration.rst index 56d9913a3370..f56cc79a9e83 100644 --- a/Documentation/firmware-guide/acpi/enumeration.rst +++ b/Documentation/firmware-guide/acpi/enumeration.rst @@ -64,6 +64,19 @@ If the driver needs to perform more complex initialization like getting and configuring GPIOs it can get its ACPI handle and extract this information from ACPI tables. +ACPI bus + + +Historically some devices not connected to any bus were represented as ACPI +devices, and had to implement ACPI driver. This is not a preferred way for new +drivers. As explained above devices not connected to any bus should implement +platform driver. ACPI device would be created during enumeration nonetheless, +and would be accessible through ACPI_COMPANION() macro, and the ACPI handle would +be accessible through ACPI_HANDLE() macro. ACPI device is meant to describe +information related to ACPI entry e.g. handle of the ACPI entry. Think - +ACPI device interfaces with the FW, and the platform device with the rest of +the system. + DMA support === -- 2.41.0
[PATCH v1 0/9] Replace acpi_driver with platform_driver
Currently there is a number of drivers that somewhat incorrectly register themselves as acpi_driver, while they should be registering as platform_drivers. acpi_device was never meant to represent actual device, it only represents an entry in the ACPI table and ACPI driver should be treated as a driver for the ACPI entry of the particular device, not the device itself. Drivers of the devices itself should register as platform_drivers. Replace acpi_driver with platform_driver for all relevant drivers in drivers/acpi directory. This is just the first part of the change, as many acpi_drivers are present in many files outside of drivers/acpi directory. Additionally during internal review we've decided that it's best to send the relevant patches sequentially, in order not to overwhelm the reviewers. So I'm starting with NFIT and AC drivers. This change is possible thanks to recent .notify callback removal in drivers/acpi [1]. So flow for the remaining acpi_drivers will look like this: 1) Remove .notify callback with separate patchset for drivers/platform and other outstanding places like drivers/hwmon, drivers/iio etc. 2) Replace acpi_driver with platform_driver, as aside for .notify callback they're mostly functionally compatible. Most of the patches in this series could be considered independent, and could be merged separately, with a notable exception of the very first patch in this series that changes notify installer wrappers to be more generic. I decided it would be best not to force the user of this wrappers to pass any specific structure, like acpi_device or platform_device. It makes sense as it is very often the case that drivers declare their own private structure which gets allocated during the .probe() callback, and become the heart of the driver. In that case it makes much more sense to pass this structure to notify handler. Additionally some drivers, like acpi_video that already have custom notify handlers do that already. Link: https://lore.kernel.org/linux-acpi/20230703080252.2899090-1-michal.wilczyn...@intel.com/ [1] Michal Wilczynski (9): ACPI: bus: Make notify wrappers more generic docs: firmware-guide: ACPI: Clarify ACPI bus concepts ACPI: AC: Remove unnecessary checks ACPI: AC: Use string_choices API instead of ternary operator ACPI: AC: Replace acpi_driver with platform_driver ACPI: AC: Rename ACPI device from device to adev ACPI: NFIT: Replace acpi_driver with platform_driver ACPI: NFIT: Remove redundant call to to_acpi_dev() ACPI: NFIT: Don't use KBUILD_MODNAME for driver name .../firmware-guide/acpi/enumeration.rst | 13 +++ drivers/acpi/ac.c | 108 -- drivers/acpi/acpi_video.c | 6 +- drivers/acpi/battery.c| 6 +- drivers/acpi/bus.c| 14 +-- drivers/acpi/hed.c| 6 +- drivers/acpi/nfit/core.c | 42 +++ drivers/acpi/thermal.c| 6 +- include/acpi/acpi_bus.h | 9 +- 9 files changed, 103 insertions(+), 107 deletions(-) -- 2.41.0
[PATCH v7 9/9] acpi/thermal: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
While at it, fix whitespaces in .remove() callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/thermal.c | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index f9f6ebb08fdb..97858ad59d68 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -825,8 +825,9 @@ static void acpi_queue_thermal_check(struct acpi_thermal
*tz)
queue_work(acpi_thermal_pm_queue, >thermal_check_work);
}
-static void acpi_thermal_notify(struct acpi_device *device, u32 event)
+static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_thermal *tz = acpi_driver_data(device);
if (!tz)
@@ -997,11 +998,21 @@ static int acpi_thermal_add(struct acpi_device *device)
pr_info("%s [%s] (%ld C)\n", acpi_device_name(device),
acpi_device_bid(device),
deci_kelvin_to_celsius(tz->temperature));
- goto end;
+ result = acpi_dev_install_notify_handler(device,
+ACPI_DEVICE_NOTIFY,
+acpi_thermal_notify);
+ if (result)
+ goto flush_wq;
+
+ return 0;
+
+flush_wq:
+ flush_workqueue(acpi_thermal_pm_queue);
+ acpi_thermal_unregister_thermal_zone(tz);
free_memory:
kfree(tz);
-end:
+
return result;
}
@@ -1012,10 +1023,15 @@ static void acpi_thermal_remove(struct acpi_device
*device)
if (!device || !acpi_driver_data(device))
return;
- flush_workqueue(acpi_thermal_pm_queue);
tz = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_thermal_notify);
+
+ flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
+
kfree(tz);
}
@@ -1078,7 +1094,6 @@ static struct acpi_driver acpi_thermal_driver = {
.ops = {
.add = acpi_thermal_add,
.remove = acpi_thermal_remove,
- .notify = acpi_thermal_notify,
},
.drv.pm = _thermal_pm,
};
--
2.41.0
[PATCH v7 8/9] acpi/nfit: Remove unnecessary .remove callback
Nfit driver doesn't use .remove() callback and provide an empty function
as it's .remove() callback. Remove empty acpi_nfit_remove() and
initialization of callback.
Suggested-by: Dan Williams
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 124e928647d3..16bf17a3d6ff 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3402,11 +3402,6 @@ static int acpi_nfit_add(struct acpi_device *adev)
adev);
}
-static void acpi_nfit_remove(struct acpi_device *adev)
-{
- /* see acpi_nfit_unregister */
-}
-
static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
{
struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(dev);
@@ -3488,7 +3483,6 @@ static struct acpi_driver acpi_nfit_driver = {
.ids = acpi_nfit_ids,
.ops = {
.add = acpi_nfit_add,
- .remove = acpi_nfit_remove,
},
};
--
2.41.0
[PATCH v7 7/9] acpi/nfit: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of
acpi_nfit_shutdown(). Change arguments passed to the notify function to
match with what's required by acpi_dev_install_notify_handler(). Remove
.notify callback initialization in acpi_driver.
Introduce a new devm action acpi_nfit_remove_notify_handler.
Move acpi_nfit_notify() upwards in the file, so it can be used inside
acpi_nfit_add() and acpi_nfit_remove_notify_handler().
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 41 +++-
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 07204d482968..124e928647d3 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3282,6 +3282,24 @@ static void acpi_nfit_put_table(void *table)
acpi_put_table(table);
}
+static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
+{
+ struct acpi_device *adev = data;
+
+ device_lock(>dev);
+ __acpi_nfit_notify(>dev, handle, event);
+ device_unlock(>dev);
+}
+
+static void acpi_nfit_remove_notify_handler(void *data)
+{
+ struct acpi_device *adev = data;
+
+ acpi_dev_remove_notify_handler(adev,
+ ACPI_DEVICE_NOTIFY,
+ acpi_nfit_notify);
+}
+
void acpi_nfit_shutdown(void *data)
{
struct acpi_nfit_desc *acpi_desc = data;
@@ -3368,7 +3386,20 @@ static int acpi_nfit_add(struct acpi_device *adev)
if (rc)
return rc;
- return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+
+ rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+ if (rc)
+ return rc;
+
+ rc = acpi_dev_install_notify_handler(adev,
+ACPI_DEVICE_NOTIFY,
+acpi_nfit_notify);
+ if (rc)
+ return rc;
+
+ return devm_add_action_or_reset(dev,
+ acpi_nfit_remove_notify_handler,
+ adev);
}
static void acpi_nfit_remove(struct acpi_device *adev)
@@ -3446,13 +3477,6 @@ void __acpi_nfit_notify(struct device *dev, acpi_handle
handle, u32 event)
}
EXPORT_SYMBOL_GPL(__acpi_nfit_notify);
-static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
-{
- device_lock(>dev);
- __acpi_nfit_notify(>dev, adev->handle, event);
- device_unlock(>dev);
-}
-
static const struct acpi_device_id acpi_nfit_ids[] = {
{ "ACPI0012", 0 },
{ "", 0 },
@@ -3465,7 +3489,6 @@ static struct acpi_driver acpi_nfit_driver = {
.ops = {
.add = acpi_nfit_add,
.remove = acpi_nfit_remove,
- .notify = acpi_nfit_notify,
},
};
--
2.41.0
[PATCH v7 6/9] acpi/hed: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/hed.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/hed.c b/drivers/acpi/hed.c
index 78d44e3fe129..8f54560c6d1c 100644
--- a/drivers/acpi/hed.c
+++ b/drivers/acpi/hed.c
@@ -42,22 +42,34 @@ EXPORT_SYMBOL_GPL(unregister_acpi_hed_notifier);
* it is used by HEST Generic Hardware Error Source with notify type
* SCI.
*/
-static void acpi_hed_notify(struct acpi_device *device, u32 event)
+static void acpi_hed_notify(acpi_handle handle, u32 event, void *data)
{
blocking_notifier_call_chain(_hed_notify_list, 0, NULL);
}
static int acpi_hed_add(struct acpi_device *device)
{
+ int err;
+
/* Only one hardware error device */
if (hed_handle)
return -EINVAL;
hed_handle = device->handle;
- return 0;
+
+ err = acpi_dev_install_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_hed_notify);
+ if (err)
+ hed_handle = NULL;
+
+ return err;
}
static void acpi_hed_remove(struct acpi_device *device)
{
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_hed_notify);
hed_handle = NULL;
}
@@ -68,7 +80,6 @@ static struct acpi_driver acpi_hed_driver = {
.ops = {
.add = acpi_hed_add,
.remove = acpi_hed_remove,
- .notify = acpi_hed_notify,
},
};
module_acpi_driver(acpi_hed_driver);
--
2.41.0
[PATCH v7 5/9] acpi/battery: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
While at it, fix lack of whitespaces in .remove() callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/battery.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 9c67ed02d797..4c634a4c32dd 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1034,8 +1034,9 @@ static void acpi_battery_refresh(struct acpi_battery
*battery)
}
/* Driver Interface */
-static void acpi_battery_notify(struct acpi_device *device, u32 event)
+static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_battery *battery = acpi_driver_data(device);
struct power_supply *old;
@@ -1212,13 +1213,23 @@ static int acpi_battery_add(struct acpi_device *device)
device_init_wakeup(>dev, 1);
- return result;
+ result = acpi_dev_install_notify_handler(device,
+ACPI_ALL_NOTIFY,
+acpi_battery_notify);
+ if (result)
+ goto fail_pm;
+
+ return 0;
+fail_pm:
+ device_init_wakeup(>dev, 0);
+ unregister_pm_notifier(>pm_nb);
fail:
sysfs_remove_battery(battery);
mutex_destroy(>lock);
mutex_destroy(>sysfs_lock);
kfree(battery);
+
return result;
}
@@ -1228,10 +1239,17 @@ static void acpi_battery_remove(struct acpi_device
*device)
if (!device || !acpi_driver_data(device))
return;
- device_init_wakeup(>dev, 0);
+
battery = acpi_driver_data(device);
+
+ acpi_dev_remove_notify_handler(device,
+ ACPI_ALL_NOTIFY,
+ acpi_battery_notify);
+
+ device_init_wakeup(>dev, 0);
unregister_pm_notifier(>pm_nb);
sysfs_remove_battery(battery);
+
mutex_destroy(>lock);
mutex_destroy(>sysfs_lock);
kfree(battery);
@@ -1264,11 +1282,9 @@ static struct acpi_driver acpi_battery_driver = {
.name = "battery",
.class = ACPI_BATTERY_CLASS,
.ids = battery_device_ids,
- .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
.ops = {
.add = acpi_battery_add,
.remove = acpi_battery_remove,
- .notify = acpi_battery_notify,
},
.drv.pm = _battery_pm,
};
--
2.41.0
[PATCH v7 4/9] acpi/video: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/acpi_video.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 62f4364e4460..168bb43e0c65 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -77,7 +77,7 @@ static DEFINE_MUTEX(video_list_lock);
static LIST_HEAD(video_bus_head);
static int acpi_video_bus_add(struct acpi_device *device);
static void acpi_video_bus_remove(struct acpi_device *device);
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data);
/*
* Indices in the _BCL method response: the first two items are special,
@@ -104,7 +104,6 @@ static struct acpi_driver acpi_video_bus = {
.ops = {
.add = acpi_video_bus_add,
.remove = acpi_video_bus_remove,
- .notify = acpi_video_bus_notify,
},
};
@@ -1527,8 +1526,9 @@ static int acpi_video_bus_stop_devices(struct
acpi_video_bus *video)
acpi_osi_is_win8() ? 0 : 1);
}
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_video_bus *video = acpi_driver_data(device);
struct input_dev *input;
int keycode = 0;
@@ -2053,8 +2053,20 @@ static int acpi_video_bus_add(struct acpi_device *device)
acpi_video_bus_add_notify_handler(video);
+ error = acpi_dev_install_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_video_bus_notify);
+ if (error)
+ goto err_remove;
+
return 0;
+err_remove:
+ mutex_lock(_list_lock);
+ list_del(>entry);
+ mutex_unlock(_list_lock);
+ acpi_video_bus_remove_notify_handler(video);
+ acpi_video_bus_unregister_backlight(video);
err_put_video:
acpi_video_bus_put_devices(video);
kfree(video->attached_array);
@@ -2075,6 +2087,10 @@ static void acpi_video_bus_remove(struct acpi_device
*device)
video = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_video_bus_notify);
+
mutex_lock(_list_lock);
list_del(>entry);
mutex_unlock(_list_lock);
--
2.41.0
[PATCH v7 3/9] acpi/ac: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 1ace70b831cd..f6feff1f3118 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -34,7 +34,7 @@ MODULE_LICENSE("GPL");
static int acpi_ac_add(struct acpi_device *device);
static void acpi_ac_remove(struct acpi_device *device);
-static void acpi_ac_notify(struct acpi_device *device, u32 event);
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
static const struct acpi_device_id ac_device_ids[] = {
{"ACPI0003", 0},
@@ -54,11 +54,9 @@ static struct acpi_driver acpi_ac_driver = {
.name = "ac",
.class = ACPI_AC_CLASS,
.ids = ac_device_ids,
- .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
.ops = {
.add = acpi_ac_add,
.remove = acpi_ac_remove,
- .notify = acpi_ac_notify,
},
.drv.pm = _ac_pm,
};
@@ -128,8 +126,9 @@ static enum power_supply_property ac_props[] = {
};
/* Driver Model */
-static void acpi_ac_notify(struct acpi_device *device, u32 event)
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_ac *ac = acpi_driver_data(device);
if (!ac)
@@ -235,7 +234,7 @@ static int acpi_ac_add(struct acpi_device *device)
result = acpi_ac_get_state(ac);
if (result)
- goto end;
+ goto err_release_ac;
psy_cfg.drv_data = ac;
@@ -248,7 +247,7 @@ static int acpi_ac_add(struct acpi_device *device)
>charger_desc, _cfg);
if (IS_ERR(ac->charger)) {
result = PTR_ERR(ac->charger);
- goto end;
+ goto err_release_ac;
}
pr_info("%s [%s] (%s)\n", acpi_device_name(device),
@@ -256,9 +255,20 @@ static int acpi_ac_add(struct acpi_device *device)
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
-end:
+
+ result = acpi_dev_install_notify_handler(device,
+ACPI_ALL_NOTIFY,
+acpi_ac_notify);
if (result)
- kfree(ac);
+ goto err_unregister;
+
+ return 0;
+
+err_unregister:
+ power_supply_unregister(ac->charger);
+ unregister_acpi_notifier(>battery_nb);
+err_release_ac:
+ kfree(ac);
return result;
}
@@ -297,6 +307,9 @@ static void acpi_ac_remove(struct acpi_device *device)
ac = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_ALL_NOTIFY,
+ acpi_ac_notify);
power_supply_unregister(ac->charger);
unregister_acpi_notifier(>battery_nb);
--
2.41.0
[PATCH v7 1/9] acpi/bus: Introduce wrappers for ACPICA event handler install/remove
Introduce new acpi_dev_install_notify_handler() and
acpi_dev_remove_notify_handler(). Those functions are replacing old
installers, and after all drivers switch to the new model, old installers
will be removed.
Make acpi_dev_install_notify_handler() and acpi_dev_remove_notify_handler()
non-static, and export symbols. This will allow the drivers to call them
directly, instead of relying on .notify callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/bus.c | 26 ++
include/acpi/acpi_bus.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 20cdfb37da79..2d6f1f45d44e 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -554,6 +554,32 @@ static void acpi_device_remove_notify_handler(struct
acpi_device *device,
acpi_os_wait_events_complete();
}
+int acpi_dev_install_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler)
+{
+ acpi_status status;
+
+ status = acpi_install_notify_handler(adev->handle,
+handler_type,
+handler,
+adev);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_dev_install_notify_handler);
+
+void acpi_dev_remove_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler)
+{
+ acpi_remove_notify_handler(adev->handle, handler_type, handler);
+ acpi_os_wait_events_complete();
+}
+EXPORT_SYMBOL(acpi_dev_remove_notify_handler);
+
/* Handle events targeting \_SB device (at present only graceful shutdown) */
#define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c941d99162c0..23fbe4a16972 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -515,6 +515,12 @@ void acpi_bus_private_data_handler(acpi_handle, void *);
int acpi_bus_get_private_data(acpi_handle, void **);
int acpi_bus_attach_private_data(acpi_handle, void *);
void acpi_bus_detach_private_data(acpi_handle);
+int acpi_dev_install_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler);
+void acpi_dev_remove_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler);
extern int acpi_notifier_call_chain(struct acpi_device *, u32, u32);
extern int register_acpi_notifier(struct notifier_block *);
extern int unregister_acpi_notifier(struct notifier_block *);
--
2.41.0
[PATCH v7 2/9] acpi/bus: Set driver_data to NULL every time .add() fails
Most drivers set driver_data during .add() callback, but usually
they don't set it back to NULL in case of a failure. Set driver_data to
NULL in acpi_device_probe() to avoid code duplication.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 2d6f1f45d44e..c087fd6e8398 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1014,8 +1014,10 @@ static int acpi_device_probe(struct device *dev)
return -ENOSYS;
ret = acpi_drv->ops.add(acpi_dev);
- if (ret)
+ if (ret) {
+ acpi_dev->driver_data = NULL;
return ret;
+ }
pr_debug("Driver [%s] successfully bound to device [%s]\n",
acpi_drv->name, acpi_dev->pnp.bus_id);
--
2.41.0
[PATCH v7 0/9] Remove .notify callback in acpi_device_ops
*** IMPORTANT *** This is part 1 - only drivers in acpi directory to ease up review process. Rest of the drivers will be handled in separate patchsets. Currently drivers support ACPI event handlers by defining .notify callback in acpi_device_ops. This solution is suboptimal as event handler installer installs intermediary function acpi_notify_device as a handler in every driver. Also this approach requires extra variable 'flags' for specifying event types that the driver want to subscribe to. Additionally this is a pre-work required to align acpi_driver with platform_driver and eventually replace acpi_driver with platform_driver. Remove .notify callback from the acpi_device_ops. Replace it with each driver installing and removing it's event handlers. This is part 1 - only drivers in acpi directory to ease up review process. v7: - fix warning by declaring acpi_nfit_remove_notify_handler() as static v6: - fixed unnecessary RCT in all drivers, as it's not a purpose of this patch series - changed error label names to simplify them - dropped commit that remove a comma - squashed commit moving code for nfit - improved nfit driver to use devm instead of .remove() - re-based as Rafael changes [1] are merged already v5: - rebased on top of Rafael changes [1], they're not merged yet - fixed rollback in multiple drivers so they don't leak resources on failure - made this part 1, meaning only drivers in acpi directory, rest of the drivers will be handled in separate patchsets to ease up review v4: - added one commit for previously missed driver sony-laptop, refactored return statements, added NULL check for event installer v3: - lkp still reported some failures for eeepc, fujitsu and toshiba_bluetooth, fix those v2: - fix compilation errors for drivers [1]: https://lore.kernel.org/linux-acpi/1847933.atdPhlSkOF@kreacher/ Michal Wilczynski (9): acpi/bus: Introduce wrappers for ACPICA event handler install/remove acpi/bus: Set driver_data to NULL every time .add() fails acpi/ac: Move handler installing logic to driver acpi/video: Move handler installing logic to driver acpi/battery: Move handler installing logic to driver acpi/hed: Move handler installing logic to driver acpi/nfit: Move handler installing logic to driver acpi/nfit: Remove unnecessary .remove callback acpi/thermal: Move handler installing logic to driver drivers/acpi/ac.c | 29 ++--- drivers/acpi/acpi_video.c | 22 --- drivers/acpi/battery.c| 26 +- drivers/acpi/bus.c| 30 +- drivers/acpi/hed.c| 17 --- drivers/acpi/nfit/core.c | 45 +++ drivers/acpi/thermal.c| 25 +- include/acpi/acpi_bus.h | 6 ++ 8 files changed, 161 insertions(+), 39 deletions(-) -- 2.41.0
[PATCH v6 9/9] acpi/thermal: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
While at it, fix whitespaces in .remove() callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/thermal.c | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index f9f6ebb08fdb..97858ad59d68 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -825,8 +825,9 @@ static void acpi_queue_thermal_check(struct acpi_thermal
*tz)
queue_work(acpi_thermal_pm_queue, >thermal_check_work);
}
-static void acpi_thermal_notify(struct acpi_device *device, u32 event)
+static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_thermal *tz = acpi_driver_data(device);
if (!tz)
@@ -997,11 +998,21 @@ static int acpi_thermal_add(struct acpi_device *device)
pr_info("%s [%s] (%ld C)\n", acpi_device_name(device),
acpi_device_bid(device),
deci_kelvin_to_celsius(tz->temperature));
- goto end;
+ result = acpi_dev_install_notify_handler(device,
+ACPI_DEVICE_NOTIFY,
+acpi_thermal_notify);
+ if (result)
+ goto flush_wq;
+
+ return 0;
+
+flush_wq:
+ flush_workqueue(acpi_thermal_pm_queue);
+ acpi_thermal_unregister_thermal_zone(tz);
free_memory:
kfree(tz);
-end:
+
return result;
}
@@ -1012,10 +1023,15 @@ static void acpi_thermal_remove(struct acpi_device
*device)
if (!device || !acpi_driver_data(device))
return;
- flush_workqueue(acpi_thermal_pm_queue);
tz = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_thermal_notify);
+
+ flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
+
kfree(tz);
}
@@ -1078,7 +1094,6 @@ static struct acpi_driver acpi_thermal_driver = {
.ops = {
.add = acpi_thermal_add,
.remove = acpi_thermal_remove,
- .notify = acpi_thermal_notify,
},
.drv.pm = _thermal_pm,
};
--
2.41.0
[PATCH v6 8/9] acpi/nfit: Remove unnecessary .remove callback
Nfit driver doesn't use .remove() callback and provide an empty function
as it's .remove() callback. Remove empty acpi_nfit_remove() and
initialization of callback.
Suggested-by: Dan Williams
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index ee2365a80aa1..daaea23cacfd 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3402,11 +3402,6 @@ static int acpi_nfit_add(struct acpi_device *adev)
adev);
}
-static void acpi_nfit_remove(struct acpi_device *adev)
-{
- /* see acpi_nfit_unregister */
-}
-
static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
{
struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(dev);
@@ -3488,7 +3483,6 @@ static struct acpi_driver acpi_nfit_driver = {
.ids = acpi_nfit_ids,
.ops = {
.add = acpi_nfit_add,
- .remove = acpi_nfit_remove,
},
};
--
2.41.0
[PATCH v6 7/9] acpi/nfit: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of
acpi_nfit_shutdown(). Change arguments passed to the notify function to
match with what's required by acpi_dev_install_notify_handler(). Remove
.notify callback initialization in acpi_driver.
Introduce a new devm action acpi_nfit_remove_notify_handler.
Move acpi_nfit_notify() upwards in the file, so it can be used inside
acpi_nfit_add() and acpi_nfit_remove_notify_handler().
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 41 +++-
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 07204d482968..ee2365a80aa1 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3282,6 +3282,24 @@ static void acpi_nfit_put_table(void *table)
acpi_put_table(table);
}
+static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
+{
+ struct acpi_device *adev = data;
+
+ device_lock(>dev);
+ __acpi_nfit_notify(>dev, handle, event);
+ device_unlock(>dev);
+}
+
+void acpi_nfit_remove_notify_handler(void *data)
+{
+ struct acpi_device *adev = data;
+
+ acpi_dev_remove_notify_handler(adev,
+ ACPI_DEVICE_NOTIFY,
+ acpi_nfit_notify);
+}
+
void acpi_nfit_shutdown(void *data)
{
struct acpi_nfit_desc *acpi_desc = data;
@@ -3368,7 +3386,20 @@ static int acpi_nfit_add(struct acpi_device *adev)
if (rc)
return rc;
- return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+
+ rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+ if (rc)
+ return rc;
+
+ rc = acpi_dev_install_notify_handler(adev,
+ACPI_DEVICE_NOTIFY,
+acpi_nfit_notify);
+ if (rc)
+ return rc;
+
+ return devm_add_action_or_reset(dev,
+ acpi_nfit_remove_notify_handler,
+ adev);
}
static void acpi_nfit_remove(struct acpi_device *adev)
@@ -3446,13 +3477,6 @@ void __acpi_nfit_notify(struct device *dev, acpi_handle
handle, u32 event)
}
EXPORT_SYMBOL_GPL(__acpi_nfit_notify);
-static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
-{
- device_lock(>dev);
- __acpi_nfit_notify(>dev, adev->handle, event);
- device_unlock(>dev);
-}
-
static const struct acpi_device_id acpi_nfit_ids[] = {
{ "ACPI0012", 0 },
{ "", 0 },
@@ -3465,7 +3489,6 @@ static struct acpi_driver acpi_nfit_driver = {
.ops = {
.add = acpi_nfit_add,
.remove = acpi_nfit_remove,
- .notify = acpi_nfit_notify,
},
};
--
2.41.0
[PATCH v6 6/9] acpi/hed: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/hed.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/hed.c b/drivers/acpi/hed.c
index 78d44e3fe129..8f54560c6d1c 100644
--- a/drivers/acpi/hed.c
+++ b/drivers/acpi/hed.c
@@ -42,22 +42,34 @@ EXPORT_SYMBOL_GPL(unregister_acpi_hed_notifier);
* it is used by HEST Generic Hardware Error Source with notify type
* SCI.
*/
-static void acpi_hed_notify(struct acpi_device *device, u32 event)
+static void acpi_hed_notify(acpi_handle handle, u32 event, void *data)
{
blocking_notifier_call_chain(_hed_notify_list, 0, NULL);
}
static int acpi_hed_add(struct acpi_device *device)
{
+ int err;
+
/* Only one hardware error device */
if (hed_handle)
return -EINVAL;
hed_handle = device->handle;
- return 0;
+
+ err = acpi_dev_install_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_hed_notify);
+ if (err)
+ hed_handle = NULL;
+
+ return err;
}
static void acpi_hed_remove(struct acpi_device *device)
{
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_hed_notify);
hed_handle = NULL;
}
@@ -68,7 +80,6 @@ static struct acpi_driver acpi_hed_driver = {
.ops = {
.add = acpi_hed_add,
.remove = acpi_hed_remove,
- .notify = acpi_hed_notify,
},
};
module_acpi_driver(acpi_hed_driver);
--
2.41.0
[PATCH v6 5/9] acpi/battery: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
While at it, fix lack of whitespaces in .remove() callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/battery.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 9c67ed02d797..4c634a4c32dd 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1034,8 +1034,9 @@ static void acpi_battery_refresh(struct acpi_battery
*battery)
}
/* Driver Interface */
-static void acpi_battery_notify(struct acpi_device *device, u32 event)
+static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_battery *battery = acpi_driver_data(device);
struct power_supply *old;
@@ -1212,13 +1213,23 @@ static int acpi_battery_add(struct acpi_device *device)
device_init_wakeup(>dev, 1);
- return result;
+ result = acpi_dev_install_notify_handler(device,
+ACPI_ALL_NOTIFY,
+acpi_battery_notify);
+ if (result)
+ goto fail_pm;
+
+ return 0;
+fail_pm:
+ device_init_wakeup(>dev, 0);
+ unregister_pm_notifier(>pm_nb);
fail:
sysfs_remove_battery(battery);
mutex_destroy(>lock);
mutex_destroy(>sysfs_lock);
kfree(battery);
+
return result;
}
@@ -1228,10 +1239,17 @@ static void acpi_battery_remove(struct acpi_device
*device)
if (!device || !acpi_driver_data(device))
return;
- device_init_wakeup(>dev, 0);
+
battery = acpi_driver_data(device);
+
+ acpi_dev_remove_notify_handler(device,
+ ACPI_ALL_NOTIFY,
+ acpi_battery_notify);
+
+ device_init_wakeup(>dev, 0);
unregister_pm_notifier(>pm_nb);
sysfs_remove_battery(battery);
+
mutex_destroy(>lock);
mutex_destroy(>sysfs_lock);
kfree(battery);
@@ -1264,11 +1282,9 @@ static struct acpi_driver acpi_battery_driver = {
.name = "battery",
.class = ACPI_BATTERY_CLASS,
.ids = battery_device_ids,
- .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
.ops = {
.add = acpi_battery_add,
.remove = acpi_battery_remove,
- .notify = acpi_battery_notify,
},
.drv.pm = _battery_pm,
};
--
2.41.0
[PATCH v6 4/9] acpi/video: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/acpi_video.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 62f4364e4460..168bb43e0c65 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -77,7 +77,7 @@ static DEFINE_MUTEX(video_list_lock);
static LIST_HEAD(video_bus_head);
static int acpi_video_bus_add(struct acpi_device *device);
static void acpi_video_bus_remove(struct acpi_device *device);
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data);
/*
* Indices in the _BCL method response: the first two items are special,
@@ -104,7 +104,6 @@ static struct acpi_driver acpi_video_bus = {
.ops = {
.add = acpi_video_bus_add,
.remove = acpi_video_bus_remove,
- .notify = acpi_video_bus_notify,
},
};
@@ -1527,8 +1526,9 @@ static int acpi_video_bus_stop_devices(struct
acpi_video_bus *video)
acpi_osi_is_win8() ? 0 : 1);
}
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_video_bus *video = acpi_driver_data(device);
struct input_dev *input;
int keycode = 0;
@@ -2053,8 +2053,20 @@ static int acpi_video_bus_add(struct acpi_device *device)
acpi_video_bus_add_notify_handler(video);
+ error = acpi_dev_install_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_video_bus_notify);
+ if (error)
+ goto err_remove;
+
return 0;
+err_remove:
+ mutex_lock(_list_lock);
+ list_del(>entry);
+ mutex_unlock(_list_lock);
+ acpi_video_bus_remove_notify_handler(video);
+ acpi_video_bus_unregister_backlight(video);
err_put_video:
acpi_video_bus_put_devices(video);
kfree(video->attached_array);
@@ -2075,6 +2087,10 @@ static void acpi_video_bus_remove(struct acpi_device
*device)
video = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_video_bus_notify);
+
mutex_lock(_list_lock);
list_del(>entry);
mutex_unlock(_list_lock);
--
2.41.0
[PATCH v6 3/9] acpi/ac: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_dev_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 1ace70b831cd..f6feff1f3118 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -34,7 +34,7 @@ MODULE_LICENSE("GPL");
static int acpi_ac_add(struct acpi_device *device);
static void acpi_ac_remove(struct acpi_device *device);
-static void acpi_ac_notify(struct acpi_device *device, u32 event);
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
static const struct acpi_device_id ac_device_ids[] = {
{"ACPI0003", 0},
@@ -54,11 +54,9 @@ static struct acpi_driver acpi_ac_driver = {
.name = "ac",
.class = ACPI_AC_CLASS,
.ids = ac_device_ids,
- .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
.ops = {
.add = acpi_ac_add,
.remove = acpi_ac_remove,
- .notify = acpi_ac_notify,
},
.drv.pm = _ac_pm,
};
@@ -128,8 +126,9 @@ static enum power_supply_property ac_props[] = {
};
/* Driver Model */
-static void acpi_ac_notify(struct acpi_device *device, u32 event)
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
+ struct acpi_device *device = data;
struct acpi_ac *ac = acpi_driver_data(device);
if (!ac)
@@ -235,7 +234,7 @@ static int acpi_ac_add(struct acpi_device *device)
result = acpi_ac_get_state(ac);
if (result)
- goto end;
+ goto err_release_ac;
psy_cfg.drv_data = ac;
@@ -248,7 +247,7 @@ static int acpi_ac_add(struct acpi_device *device)
>charger_desc, _cfg);
if (IS_ERR(ac->charger)) {
result = PTR_ERR(ac->charger);
- goto end;
+ goto err_release_ac;
}
pr_info("%s [%s] (%s)\n", acpi_device_name(device),
@@ -256,9 +255,20 @@ static int acpi_ac_add(struct acpi_device *device)
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
-end:
+
+ result = acpi_dev_install_notify_handler(device,
+ACPI_ALL_NOTIFY,
+acpi_ac_notify);
if (result)
- kfree(ac);
+ goto err_unregister;
+
+ return 0;
+
+err_unregister:
+ power_supply_unregister(ac->charger);
+ unregister_acpi_notifier(>battery_nb);
+err_release_ac:
+ kfree(ac);
return result;
}
@@ -297,6 +307,9 @@ static void acpi_ac_remove(struct acpi_device *device)
ac = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_ALL_NOTIFY,
+ acpi_ac_notify);
power_supply_unregister(ac->charger);
unregister_acpi_notifier(>battery_nb);
--
2.41.0
[PATCH v6 2/9] acpi/bus: Set driver_data to NULL every time .add() fails
Most drivers set driver_data during .add() callback, but usually
they don't set it back to NULL in case of a failure. Set driver_data to
NULL in acpi_device_probe() to avoid code duplication.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 2d6f1f45d44e..c087fd6e8398 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1014,8 +1014,10 @@ static int acpi_device_probe(struct device *dev)
return -ENOSYS;
ret = acpi_drv->ops.add(acpi_dev);
- if (ret)
+ if (ret) {
+ acpi_dev->driver_data = NULL;
return ret;
+ }
pr_debug("Driver [%s] successfully bound to device [%s]\n",
acpi_drv->name, acpi_dev->pnp.bus_id);
--
2.41.0
[PATCH v6 1/9] acpi/bus: Introduce wrappers for ACPICA event handler install/remove
Introduce new acpi_dev_install_notify_handler() and
acpi_dev_remove_notify_handler(). Those functions are replacing old
installers, and after all drivers switch to the new model, old installers
will be removed.
Make acpi_dev_install_notify_handler() and acpi_dev_remove_notify_handler()
non-static, and export symbols. This will allow the drivers to call them
directly, instead of relying on .notify callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/bus.c | 26 ++
include/acpi/acpi_bus.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 20cdfb37da79..2d6f1f45d44e 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -554,6 +554,32 @@ static void acpi_device_remove_notify_handler(struct
acpi_device *device,
acpi_os_wait_events_complete();
}
+int acpi_dev_install_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler)
+{
+ acpi_status status;
+
+ status = acpi_install_notify_handler(adev->handle,
+handler_type,
+handler,
+adev);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_dev_install_notify_handler);
+
+void acpi_dev_remove_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler)
+{
+ acpi_remove_notify_handler(adev->handle, handler_type, handler);
+ acpi_os_wait_events_complete();
+}
+EXPORT_SYMBOL(acpi_dev_remove_notify_handler);
+
/* Handle events targeting \_SB device (at present only graceful shutdown) */
#define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c941d99162c0..23fbe4a16972 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -515,6 +515,12 @@ void acpi_bus_private_data_handler(acpi_handle, void *);
int acpi_bus_get_private_data(acpi_handle, void **);
int acpi_bus_attach_private_data(acpi_handle, void *);
void acpi_bus_detach_private_data(acpi_handle);
+int acpi_dev_install_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler);
+void acpi_dev_remove_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler);
extern int acpi_notifier_call_chain(struct acpi_device *, u32, u32);
extern int register_acpi_notifier(struct notifier_block *);
extern int unregister_acpi_notifier(struct notifier_block *);
--
2.41.0
[PATCH v6 0/9] Remove .notify callback in acpi_device_ops
*** IMPORTANT *** This is part 1 - only drivers in acpi directory to ease up review process. Rest of the drivers will be handled in separate patchsets. Currently drivers support ACPI event handlers by defining .notify callback in acpi_device_ops. This solution is suboptimal as event handler installer installs intermediary function acpi_notify_device as a handler in every driver. Also this approach requires extra variable 'flags' for specifying event types that the driver want to subscribe to. Additionally this is a pre-work required to align acpi_driver with platform_driver and eventually replace acpi_driver with platform_driver. Remove .notify callback from the acpi_device_ops. Replace it with each driver installing and removing it's event handlers. This is part 1 - only drivers in acpi directory to ease up review process. v6: - fixed unnecessary RCT in all drivers, as it's not a purpose of this patch series - changed error label names to simplify them - dropped commit that remove a comma - squashed commit moving code for nfit - improved nfit driver to use devm instead of .remove() - re-based as Rafael changes [1] are merged already v5: - rebased on top of Rafael changes [1], they're not merged yet - fixed rollback in multiple drivers so they don't leak resources on failure - made this part 1, meaning only drivers in acpi directory, rest of the drivers will be handled in separate patchsets to ease up review v4: - added one commit for previously missed driver sony-laptop, refactored return statements, added NULL check for event installer v3: - lkp still reported some failures for eeepc, fujitsu and toshiba_bluetooth, fix those v2: - fix compilation errors for drivers [1]: https://lore.kernel.org/linux-acpi/1847933.atdPhlSkOF@kreacher/ Michal Wilczynski (9): acpi/bus: Introduce wrappers for ACPICA event handler install/remove acpi/bus: Set driver_data to NULL every time .add() fails acpi/ac: Move handler installing logic to driver acpi/video: Move handler installing logic to driver acpi/battery: Move handler installing logic to driver acpi/hed: Move handler installing logic to driver acpi/nfit: Move handler installing logic to driver acpi/nfit: Remove unnecessary .remove callback acpi/thermal: Move handler installing logic to driver drivers/acpi/ac.c | 29 ++--- drivers/acpi/acpi_video.c | 22 --- drivers/acpi/battery.c| 26 +- drivers/acpi/bus.c| 30 +- drivers/acpi/hed.c| 17 --- drivers/acpi/nfit/core.c | 45 +++ drivers/acpi/thermal.c| 25 +- include/acpi/acpi_bus.h | 6 ++ 8 files changed, 161 insertions(+), 39 deletions(-) -- 2.41.0
[PATCH v5 10/10] acpi/thermal: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
While at it, fix whitespaces in .remove() callback and move tz
assignment upwards.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/thermal.c | 28 ++--
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index f9f6ebb08fdb..84716e4b967c 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -825,9 +825,12 @@ static void acpi_queue_thermal_check(struct acpi_thermal
*tz)
queue_work(acpi_thermal_pm_queue, >thermal_check_work);
}
-static void acpi_thermal_notify(struct acpi_device *device, u32 event)
+static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_thermal *tz = acpi_driver_data(device);
+ struct acpi_device *device = data;
+ struct acpi_thermal *tz;
+
+ tz = acpi_driver_data(device);
if (!tz)
return;
@@ -997,11 +1000,20 @@ static int acpi_thermal_add(struct acpi_device *device)
pr_info("%s [%s] (%ld C)\n", acpi_device_name(device),
acpi_device_bid(device),
deci_kelvin_to_celsius(tz->temperature));
- goto end;
+ result = acpi_dev_install_notify_handler(device,
+ACPI_DEVICE_NOTIFY,
+acpi_thermal_notify);
+ if (result)
+ goto flush_wq_and_unregister;
+
+ return 0;
+
+flush_wq_and_unregister:
+ flush_workqueue(acpi_thermal_pm_queue);
+ acpi_thermal_unregister_thermal_zone(tz);
free_memory:
kfree(tz);
-end:
return result;
}
@@ -1012,10 +1024,15 @@ static void acpi_thermal_remove(struct acpi_device
*device)
if (!device || !acpi_driver_data(device))
return;
- flush_workqueue(acpi_thermal_pm_queue);
tz = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_thermal_notify);
+
+ flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
+
kfree(tz);
}
@@ -1078,7 +1095,6 @@ static struct acpi_driver acpi_thermal_driver = {
.ops = {
.add = acpi_thermal_add,
.remove = acpi_thermal_remove,
- .notify = acpi_thermal_notify,
},
.drv.pm = _thermal_pm,
};
--
2.41.0
[PATCH v5 09/10] acpi/nfit: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 95930e9d776c..a281bdfee8a0 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data)
}
EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
-static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
+static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
{
- device_lock(>dev);
- __acpi_nfit_notify(>dev, adev->handle, event);
- device_unlock(>dev);
+ struct acpi_device *device = data;
+
+ device_lock(>dev);
+ __acpi_nfit_notify(>dev, handle, event);
+ device_unlock(>dev);
}
static int acpi_nfit_add(struct acpi_device *adev)
@@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev)
if (rc)
return rc;
- return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+
+ rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+ if (rc)
+ return rc;
+
+ return acpi_dev_install_notify_handler(adev,
+ ACPI_DEVICE_NOTIFY,
+ acpi_nfit_notify);
}
static void acpi_nfit_remove(struct acpi_device *adev)
{
/* see acpi_nfit_unregister */
+
+ acpi_dev_remove_notify_handler(adev,
+ ACPI_DEVICE_NOTIFY,
+ acpi_nfit_notify);
}
static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
@@ -3465,7 +3478,6 @@ static struct acpi_driver acpi_nfit_driver = {
.ops = {
.add = acpi_nfit_add,
.remove = acpi_nfit_remove,
- .notify = acpi_nfit_notify,
},
};
--
2.41.0
[PATCH v5 08/10] acpi/nfit: Improve terminator line in acpi_nfit_ids
Currently terminator line contains redunant characters. Remove them and
also remove a comma at the end.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index aff79cbc2190..95930e9d776c 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3455,7 +3455,7 @@ EXPORT_SYMBOL_GPL(__acpi_nfit_notify);
static const struct acpi_device_id acpi_nfit_ids[] = {
{ "ACPI0012", 0 },
- { "", 0 },
+ {}
};
MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids);
--
2.41.0
[PATCH v5 07/10] acpi/nfit: Move acpi_nfit_notify() before acpi_nfit_add()
To use new style of installing event handlers acpi_nfit_notify() needs
to be known inside acpi_nfit_add(). Move acpi_nfit_notify() upwards in
the file, so it can be used inside acpi_nfit_add().
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 07204d482968..aff79cbc2190 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3312,6 +3312,13 @@ void acpi_nfit_shutdown(void *data)
}
EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
+static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
+{
+ device_lock(>dev);
+ __acpi_nfit_notify(>dev, adev->handle, event);
+ device_unlock(>dev);
+}
+
static int acpi_nfit_add(struct acpi_device *adev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -3446,13 +3453,6 @@ void __acpi_nfit_notify(struct device *dev, acpi_handle
handle, u32 event)
}
EXPORT_SYMBOL_GPL(__acpi_nfit_notify);
-static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
-{
- device_lock(>dev);
- __acpi_nfit_notify(>dev, adev->handle, event);
- device_unlock(>dev);
-}
-
static const struct acpi_device_id acpi_nfit_ids[] = {
{ "ACPI0012", 0 },
{ "", 0 },
--
2.41.0
[PATCH v5 06/10] acpi/hed: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/hed.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/hed.c b/drivers/acpi/hed.c
index 78d44e3fe129..8f54560c6d1c 100644
--- a/drivers/acpi/hed.c
+++ b/drivers/acpi/hed.c
@@ -42,22 +42,34 @@ EXPORT_SYMBOL_GPL(unregister_acpi_hed_notifier);
* it is used by HEST Generic Hardware Error Source with notify type
* SCI.
*/
-static void acpi_hed_notify(struct acpi_device *device, u32 event)
+static void acpi_hed_notify(acpi_handle handle, u32 event, void *data)
{
blocking_notifier_call_chain(_hed_notify_list, 0, NULL);
}
static int acpi_hed_add(struct acpi_device *device)
{
+ int err;
+
/* Only one hardware error device */
if (hed_handle)
return -EINVAL;
hed_handle = device->handle;
- return 0;
+
+ err = acpi_dev_install_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_hed_notify);
+ if (err)
+ hed_handle = NULL;
+
+ return err;
}
static void acpi_hed_remove(struct acpi_device *device)
{
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_hed_notify);
hed_handle = NULL;
}
@@ -68,7 +80,6 @@ static struct acpi_driver acpi_hed_driver = {
.ops = {
.add = acpi_hed_add,
.remove = acpi_hed_remove,
- .notify = acpi_hed_notify,
},
};
module_acpi_driver(acpi_hed_driver);
--
2.41.0
[PATCH v5 05/10] acpi/battery: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
While at it, fix lack of whitespaces in .remove() callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/battery.c | 30 --
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 9c67ed02d797..6337e7b1f6e1 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1034,11 +1034,14 @@ static void acpi_battery_refresh(struct acpi_battery
*battery)
}
/* Driver Interface */
-static void acpi_battery_notify(struct acpi_device *device, u32 event)
+static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_battery *battery = acpi_driver_data(device);
+ struct acpi_device *device = data;
+ struct acpi_battery *battery;
struct power_supply *old;
+ battery = acpi_driver_data(device);
+
if (!battery)
return;
old = battery->bat;
@@ -1212,13 +1215,23 @@ static int acpi_battery_add(struct acpi_device *device)
device_init_wakeup(>dev, 1);
- return result;
+ result = acpi_dev_install_notify_handler(device,
+ACPI_ALL_NOTIFY,
+acpi_battery_notify);
+ if (result)
+ goto fail_deinit_wakup_and_unregister;
+
+ return 0;
+fail_deinit_wakup_and_unregister:
+ device_init_wakeup(>dev, 0);
+ unregister_pm_notifier(>pm_nb);
fail:
sysfs_remove_battery(battery);
mutex_destroy(>lock);
mutex_destroy(>sysfs_lock);
kfree(battery);
+
return result;
}
@@ -1228,10 +1241,17 @@ static void acpi_battery_remove(struct acpi_device
*device)
if (!device || !acpi_driver_data(device))
return;
- device_init_wakeup(>dev, 0);
+
battery = acpi_driver_data(device);
+
+ acpi_dev_remove_notify_handler(device,
+ ACPI_ALL_NOTIFY,
+ acpi_battery_notify);
+
+ device_init_wakeup(>dev, 0);
unregister_pm_notifier(>pm_nb);
sysfs_remove_battery(battery);
+
mutex_destroy(>lock);
mutex_destroy(>sysfs_lock);
kfree(battery);
@@ -1264,11 +1284,9 @@ static struct acpi_driver acpi_battery_driver = {
.name = "battery",
.class = ACPI_BATTERY_CLASS,
.ids = battery_device_ids,
- .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
.ops = {
.add = acpi_battery_add,
.remove = acpi_battery_remove,
- .notify = acpi_battery_notify,
},
.drv.pm = _battery_pm,
};
--
2.41.0
[PATCH v5 04/10] acpi/video: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/acpi_video.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 62f4364e4460..60b7013d0009 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -77,7 +77,7 @@ static DEFINE_MUTEX(video_list_lock);
static LIST_HEAD(video_bus_head);
static int acpi_video_bus_add(struct acpi_device *device);
static void acpi_video_bus_remove(struct acpi_device *device);
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data);
/*
* Indices in the _BCL method response: the first two items are special,
@@ -104,7 +104,6 @@ static struct acpi_driver acpi_video_bus = {
.ops = {
.add = acpi_video_bus_add,
.remove = acpi_video_bus_remove,
- .notify = acpi_video_bus_notify,
},
};
@@ -1527,12 +1526,15 @@ static int acpi_video_bus_stop_devices(struct
acpi_video_bus *video)
acpi_osi_is_win8() ? 0 : 1);
}
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_video_bus *video = acpi_driver_data(device);
+ struct acpi_device *device = data;
+ struct acpi_video_bus *video;
struct input_dev *input;
int keycode = 0;
+ video = acpi_driver_data(device);
+
if (!video || !video->input)
return;
@@ -2053,8 +2055,20 @@ static int acpi_video_bus_add(struct acpi_device *device)
acpi_video_bus_add_notify_handler(video);
+ error = acpi_dev_install_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_video_bus_notify);
+ if (error)
+ goto err_remove_and_unregister_video;
+
return 0;
+err_remove_and_unregister_video:
+ mutex_lock(_list_lock);
+ list_del(>entry);
+ mutex_unlock(_list_lock);
+ acpi_video_bus_remove_notify_handler(video);
+ acpi_video_bus_unregister_backlight(video);
err_put_video:
acpi_video_bus_put_devices(video);
kfree(video->attached_array);
@@ -2075,6 +2089,10 @@ static void acpi_video_bus_remove(struct acpi_device
*device)
video = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_DEVICE_NOTIFY,
+ acpi_video_bus_notify);
+
mutex_lock(_list_lock);
list_del(>entry);
mutex_unlock(_list_lock);
--
2.41.0
[PATCH v5 03/10] acpi/ac: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.
Call acpi_dev_install_notify_handler() at the end of .add() callback.
Call acpi_dev_remove_notify_handler() at the beginning of .remove()
callback. Change arguments passed to the notify function to match with
what's required by acpi_install_notify_handler(). Remove .notify
callback initialization in acpi_driver.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/ac.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 1ace70b831cd..207ee3c85bad 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -34,7 +34,7 @@ MODULE_LICENSE("GPL");
static int acpi_ac_add(struct acpi_device *device);
static void acpi_ac_remove(struct acpi_device *device);
-static void acpi_ac_notify(struct acpi_device *device, u32 event);
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
static const struct acpi_device_id ac_device_ids[] = {
{"ACPI0003", 0},
@@ -54,11 +54,9 @@ static struct acpi_driver acpi_ac_driver = {
.name = "ac",
.class = ACPI_AC_CLASS,
.ids = ac_device_ids,
- .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
.ops = {
.add = acpi_ac_add,
.remove = acpi_ac_remove,
- .notify = acpi_ac_notify,
},
.drv.pm = _ac_pm,
};
@@ -128,9 +126,12 @@ static enum power_supply_property ac_props[] = {
};
/* Driver Model */
-static void acpi_ac_notify(struct acpi_device *device, u32 event)
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
{
- struct acpi_ac *ac = acpi_driver_data(device);
+ struct acpi_device *device = data;
+ struct acpi_ac *ac;
+
+ ac = acpi_driver_data(device);
if (!ac)
return;
@@ -235,7 +236,7 @@ static int acpi_ac_add(struct acpi_device *device)
result = acpi_ac_get_state(ac);
if (result)
- goto end;
+ goto err_release_ac;
psy_cfg.drv_data = ac;
@@ -248,7 +249,7 @@ static int acpi_ac_add(struct acpi_device *device)
>charger_desc, _cfg);
if (IS_ERR(ac->charger)) {
result = PTR_ERR(ac->charger);
- goto end;
+ goto err_release_ac;
}
pr_info("%s [%s] (%s)\n", acpi_device_name(device),
@@ -256,9 +257,20 @@ static int acpi_ac_add(struct acpi_device *device)
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(>battery_nb);
-end:
+
+ result = acpi_dev_install_notify_handler(device,
+ACPI_ALL_NOTIFY,
+acpi_ac_notify);
if (result)
- kfree(ac);
+ goto err_unregister;
+
+ return 0;
+
+err_unregister:
+ power_supply_unregister(ac->charger);
+ unregister_acpi_notifier(>battery_nb);
+err_release_ac:
+ kfree(ac);
return result;
}
@@ -297,6 +309,9 @@ static void acpi_ac_remove(struct acpi_device *device)
ac = acpi_driver_data(device);
+ acpi_dev_remove_notify_handler(device,
+ ACPI_ALL_NOTIFY,
+ acpi_ac_notify);
power_supply_unregister(ac->charger);
unregister_acpi_notifier(>battery_nb);
--
2.41.0
[PATCH v5 02/10] acpi/bus: Set driver_data to NULL every time .add() fails
Most drivers set driver_data during .add() callback, but usually
they don't set it back to NULL in case of a failure. Set driver_data to
NULL in acpi_device_probe() to avoid code duplication.
Signed-off-by: Michal Wilczynski
---
drivers/acpi/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 22468589c551..c1cb570c8d8c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1017,8 +1017,10 @@ static int acpi_device_probe(struct device *dev)
return -ENOSYS;
ret = acpi_drv->ops.add(acpi_dev);
- if (ret)
+ if (ret) {
+ acpi_dev->driver_data = NULL;
return ret;
+ }
pr_debug("Driver [%s] successfully bound to device [%s]\n",
acpi_drv->name, acpi_dev->pnp.bus_id);
--
2.41.0
[PATCH v5 01/10] acpi/bus: Introduce wrappers for ACPICA event handler install/remove
Introduce new acpi_dev_install_notify_handler() and
acpi_dev_remove_notify_handler(). Those functions are replacing old
installers, and after all drivers switch to the new model, old installers
will be removed.
Make acpi_dev_install_notify_handler() and acpi_dev_remove_notify_handler()
non-static, and export symbols. This will allow the drivers to call them
directly, instead of relying on .notify callback.
Suggested-by: Rafael J. Wysocki
Signed-off-by: Michal Wilczynski
---
drivers/acpi/bus.c | 26 ++
include/acpi/acpi_bus.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index b6ab3608d782..22468589c551 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -557,6 +557,32 @@ static void acpi_device_remove_notify_handler(struct
acpi_device *device,
acpi_os_wait_events_complete();
}
+int acpi_dev_install_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler)
+{
+ acpi_status status;
+
+ status = acpi_install_notify_handler(adev->handle,
+handler_type,
+handler,
+adev);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_dev_install_notify_handler);
+
+void acpi_dev_remove_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler)
+{
+ acpi_remove_notify_handler(adev->handle, handler_type, handler);
+ acpi_os_wait_events_complete();
+}
+EXPORT_SYMBOL(acpi_dev_remove_notify_handler);
+
/* Handle events targeting \_SB device (at present only graceful shutdown) */
#define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c941d99162c0..23fbe4a16972 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -515,6 +515,12 @@ void acpi_bus_private_data_handler(acpi_handle, void *);
int acpi_bus_get_private_data(acpi_handle, void **);
int acpi_bus_attach_private_data(acpi_handle, void *);
void acpi_bus_detach_private_data(acpi_handle);
+int acpi_dev_install_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler);
+void acpi_dev_remove_notify_handler(struct acpi_device *adev,
+ u32 handler_type,
+ acpi_notify_handler handler);
extern int acpi_notifier_call_chain(struct acpi_device *, u32, u32);
extern int register_acpi_notifier(struct notifier_block *);
extern int unregister_acpi_notifier(struct notifier_block *);
--
2.41.0
[PATCH v5 00/10] Remove .notify callback in acpi_device_ops
*** IMPORTANT *** This is part 1 - only drivers in acpi directory to ease up review process. Rest of the drivers will be handled in separate patchsets. Currently drivers support ACPI event handlers by defining .notify callback in acpi_device_ops. This solution is suboptimal as event handler installer installs intermediary function acpi_notify_device as a handler in every driver. Also this approach requires extra variable 'flags' for specifying event types that the driver want to subscribe to. Additionally this is a pre-work required to align acpi_driver with platform_driver and eventually replace acpi_driver with platform_driver. Remove .notify callback from the acpi_device_ops. Replace it with each driver installing and removing it's event handlers. v5: - rebased on top of Rafael changes [1], they're not merged yet - fixed rollback in multiple drivers so they don't leak resources on failure - made this part 1, meaning only drivers in acpi directory, rest of the drivers will be handled in separate patchsets to ease up review v4: - added one commit for previously missed driver sony-laptop, refactored return statements, added NULL check for event installer v3: - lkp still reported some failures for eeepc, fujitsu and toshiba_bluetooth, fix those v2: - fix compilation errors for drivers [1]: https://lore.kernel.org/linux-acpi/1847933.atdPhlSkOF@kreacher/ Michal Wilczynski (10): acpi/bus: Introduce wrappers for ACPICA event handler install/remove acpi/bus: Set driver_data to NULL every time .add() fails acpi/ac: Move handler installing logic to driver acpi/video: Move handler installing logic to driver acpi/battery: Move handler installing logic to driver acpi/hed: Move handler installing logic to driver acpi/nfit: Move acpi_nfit_notify() before acpi_nfit_add() acpi/nfit: Improve terminator line in acpi_nfit_ids acpi/nfit: Move handler installing logic to driver acpi/thermal: Move handler installing logic to driver drivers/acpi/ac.c | 33 - drivers/acpi/acpi_video.c | 26 ++ drivers/acpi/battery.c| 30 -- drivers/acpi/bus.c| 30 +- drivers/acpi/hed.c| 17 ++--- drivers/acpi/nfit/core.c | 32 ++-- drivers/acpi/thermal.c| 28 ++-- include/acpi/acpi_bus.h | 6 ++ 8 files changed, 163 insertions(+), 39 deletions(-) -- 2.41.0
