2022-08-25 16:44:58

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/5] ACPI: bus: Drop kernel doc annotation from acpi_bus_notify()

The description for acpi_bus_notify() is quite far from what
kernel doc expects. It complains about this:

Function parameter or member 'handle' not described in 'acpi_bus_notify'
Function parameter or member 'type' not described in 'acpi_bus_notify'
Function parameter or member 'data' not described in 'acpi_bus_notify'

Fix this by dropping kernel doc annotation.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 6a1476cba3d3..8e87996607ec 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -456,7 +456,7 @@ static void acpi_bus_osc_negotiate_usb_control(void)
Notification Handling
-------------------------------------------------------------------------- */

-/**
+/*
* acpi_bus_notify
* ---------------
* Callback for all 'system-level' device notifications (values 0x00-0x7F).
--
2.35.1


2022-08-25 17:02:49

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 5/5] ACPI: bus: Use the matching table, if ACPI driver has it

In case we have an ACPI driver, check its ID table for matching,
This allows to use some generic device property APIs in such
drivers.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/bus.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 3c0f2d050d47..17c98e826bde 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1049,14 +1049,30 @@ static const void *acpi_of_device_get_match_data(const struct device *dev)
return match->data;
}

+static const struct acpi_device_id *acpi_device_get_ids(const struct device *dev)
+{
+ if (dev->driver->acpi_match_table)
+ return dev->driver->acpi_match_table;
+
+ if (dev_is_acpi(dev)) {
+ struct acpi_driver *drv = to_acpi_driver(dev->driver);
+
+ if (drv->ids)
+ return drv->ids;
+ }
+
+ return NULL;
+}
+
const void *acpi_device_get_match_data(const struct device *dev)
{
+ const struct acpi_device_id *ids = acpi_device_get_ids(dev);
const struct acpi_device_id *match;

- if (!dev->driver->acpi_match_table)
+ if (!ids)
return acpi_of_device_get_match_data(dev);

- match = acpi_match_device(dev->driver->acpi_match_table, dev);
+ match = acpi_match_device(ids, dev);
if (!match)
return NULL;

--
2.35.1

2022-08-25 17:05:53

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1 5/5] ACPI: bus: Use the matching table, if ACPI driver has it

On Thu, Aug 25, 2022 at 6:41 PM Andy Shevchenko
<[email protected]> wrote:
>
> In case we have an ACPI driver, check its ID table for matching,
> This allows to use some generic device property APIs in such
> drivers.

No new provisions for ACPI drivers, please.

> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/acpi/bus.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 3c0f2d050d47..17c98e826bde 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1049,14 +1049,30 @@ static const void *acpi_of_device_get_match_data(const struct device *dev)
> return match->data;
> }
>
> +static const struct acpi_device_id *acpi_device_get_ids(const struct device *dev)
> +{
> + if (dev->driver->acpi_match_table)
> + return dev->driver->acpi_match_table;
> +
> + if (dev_is_acpi(dev)) {
> + struct acpi_driver *drv = to_acpi_driver(dev->driver);
> +
> + if (drv->ids)
> + return drv->ids;
> + }
> +
> + return NULL;
> +}
> +
> const void *acpi_device_get_match_data(const struct device *dev)
> {
> + const struct acpi_device_id *ids = acpi_device_get_ids(dev);
> const struct acpi_device_id *match;
>
> - if (!dev->driver->acpi_match_table)
> + if (!ids)
> return acpi_of_device_get_match_data(dev);
>
> - match = acpi_match_device(dev->driver->acpi_match_table, dev);
> + match = acpi_match_device(ids, dev);
> if (!match)
> return NULL;
>
> --
> 2.35.1
>

2022-08-25 17:13:18

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 3/5] ACPI: bus: Refactor acpi_bus_register_driver() to get rid of 'ret'

There is no need to have 'ret' variable in acpi_bus_register_driver().
Refactor it accordingly.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/bus.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index c4d63c594d68..607e664b7976 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -972,16 +972,14 @@ EXPORT_SYMBOL_GPL(acpi_driver_match_device);
*/
int acpi_bus_register_driver(struct acpi_driver *driver)
{
- int ret;
-
if (acpi_disabled)
return -ENODEV;
+
driver->drv.name = driver->name;
driver->drv.bus = &acpi_bus_type;
driver->drv.owner = driver->owner;

- ret = driver_register(&driver->drv);
- return ret;
+ return driver_register(&driver->drv);
}

EXPORT_SYMBOL(acpi_bus_register_driver);
--
2.35.1

2022-08-25 17:15:35

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 4/5] ACPI: bus: Move bus type operations upper in the code

Move ACPI bus type operations upper in the code, it may be used later
by ACPI device matching code. No functional change intended.

While at it, provide dev_is_acpi() macro helper as it's done for some
other bus types.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/bus.c | 250 +++++++++++++++++++++++----------------------
1 file changed, 126 insertions(+), 124 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 607e664b7976..3c0f2d050d47 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -647,6 +647,132 @@ static int __init acpi_setup_sb_notify_handler(void)
return 0;
}

+/* --------------------------------------------------------------------------
+ ACPI Bus operations
+ -------------------------------------------------------------------------- */
+
+static int acpi_bus_match(struct device *dev, struct device_driver *drv)
+{
+ struct acpi_device *acpi_dev = to_acpi_device(dev);
+ struct acpi_driver *acpi_drv = to_acpi_driver(drv);
+
+ return acpi_dev->flags.match_driver
+ && !acpi_match_device_ids(acpi_dev, acpi_drv->ids);
+}
+
+static int acpi_device_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+ return __acpi_device_uevent_modalias(to_acpi_device(dev), env);
+}
+
+static int acpi_device_probe(struct device *dev)
+{
+ struct acpi_device *acpi_dev = to_acpi_device(dev);
+ struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
+ int ret;
+
+ if (acpi_dev->handler && !acpi_is_pnp_device(acpi_dev))
+ return -EINVAL;
+
+ if (!acpi_drv->ops.add)
+ return -ENOSYS;
+
+ ret = acpi_drv->ops.add(acpi_dev);
+ if (ret)
+ return ret;
+
+ pr_debug("Driver [%s] successfully bound to device [%s]\n",
+ acpi_drv->name, acpi_dev->pnp.bus_id);
+
+ if (acpi_drv->ops.notify) {
+ ret = acpi_device_install_notify_handler(acpi_dev);
+ if (ret) {
+ if (acpi_drv->ops.remove)
+ acpi_drv->ops.remove(acpi_dev);
+
+ acpi_dev->driver_data = NULL;
+ return ret;
+ }
+ }
+
+ pr_debug("Found driver [%s] for device [%s]\n", acpi_drv->name,
+ acpi_dev->pnp.bus_id);
+
+ get_device(dev);
+ return 0;
+}
+
+static void acpi_device_remove(struct device *dev)
+{
+ struct acpi_device *acpi_dev = to_acpi_device(dev);
+ struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
+
+ if (acpi_drv->ops.notify)
+ acpi_device_remove_notify_handler(acpi_dev);
+
+ if (acpi_drv->ops.remove)
+ acpi_drv->ops.remove(acpi_dev);
+
+ acpi_dev->driver_data = NULL;
+
+ put_device(dev);
+}
+
+struct bus_type acpi_bus_type = {
+ .name = "acpi",
+ .match = acpi_bus_match,
+ .probe = acpi_device_probe,
+ .remove = acpi_device_remove,
+ .uevent = acpi_device_uevent,
+};
+
+#define dev_is_acpi(dev) ((dev)->bus == &acpi_bus_type)
+
+int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data)
+{
+ return bus_for_each_dev(&acpi_bus_type, NULL, data, fn);
+}
+EXPORT_SYMBOL_GPL(acpi_bus_for_each_dev);
+
+struct acpi_dev_walk_context {
+ int (*fn)(struct acpi_device *, void *);
+ void *data;
+};
+
+static int acpi_dev_for_one_check(struct device *dev, void *context)
+{
+ struct acpi_dev_walk_context *adwc = context;
+
+ if (!dev_is_acpi(dev))
+ return 0;
+
+ return adwc->fn(to_acpi_device(dev), adwc->data);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_for_each_child);
+
+int acpi_dev_for_each_child(struct acpi_device *adev,
+ int (*fn)(struct acpi_device *, void *), void *data)
+{
+ struct acpi_dev_walk_context adwc = {
+ .fn = fn,
+ .data = data,
+ };
+
+ return device_for_each_child(&adev->dev, &adwc, acpi_dev_for_one_check);
+}
+
+int acpi_dev_for_each_child_reverse(struct acpi_device *adev,
+ int (*fn)(struct acpi_device *, void *),
+ void *data)
+{
+ struct acpi_dev_walk_context adwc = {
+ .fn = fn,
+ .data = data,
+ };
+
+ return device_for_each_child_reverse(&adev->dev, &adwc, acpi_dev_for_one_check);
+}
+
/* --------------------------------------------------------------------------
Device Matching
-------------------------------------------------------------------------- */
@@ -998,130 +1124,6 @@ void acpi_bus_unregister_driver(struct acpi_driver *driver)

EXPORT_SYMBOL(acpi_bus_unregister_driver);

-/* --------------------------------------------------------------------------
- ACPI Bus operations
- -------------------------------------------------------------------------- */
-
-static int acpi_bus_match(struct device *dev, struct device_driver *drv)
-{
- struct acpi_device *acpi_dev = to_acpi_device(dev);
- struct acpi_driver *acpi_drv = to_acpi_driver(drv);
-
- return acpi_dev->flags.match_driver
- && !acpi_match_device_ids(acpi_dev, acpi_drv->ids);
-}
-
-static int acpi_device_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
- return __acpi_device_uevent_modalias(to_acpi_device(dev), env);
-}
-
-static int acpi_device_probe(struct device *dev)
-{
- struct acpi_device *acpi_dev = to_acpi_device(dev);
- struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
- int ret;
-
- if (acpi_dev->handler && !acpi_is_pnp_device(acpi_dev))
- return -EINVAL;
-
- if (!acpi_drv->ops.add)
- return -ENOSYS;
-
- ret = acpi_drv->ops.add(acpi_dev);
- if (ret)
- return ret;
-
- pr_debug("Driver [%s] successfully bound to device [%s]\n",
- acpi_drv->name, acpi_dev->pnp.bus_id);
-
- if (acpi_drv->ops.notify) {
- ret = acpi_device_install_notify_handler(acpi_dev);
- if (ret) {
- if (acpi_drv->ops.remove)
- acpi_drv->ops.remove(acpi_dev);
-
- acpi_dev->driver_data = NULL;
- return ret;
- }
- }
-
- pr_debug("Found driver [%s] for device [%s]\n", acpi_drv->name,
- acpi_dev->pnp.bus_id);
-
- get_device(dev);
- return 0;
-}
-
-static void acpi_device_remove(struct device *dev)
-{
- struct acpi_device *acpi_dev = to_acpi_device(dev);
- struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
-
- if (acpi_drv->ops.notify)
- acpi_device_remove_notify_handler(acpi_dev);
-
- if (acpi_drv->ops.remove)
- acpi_drv->ops.remove(acpi_dev);
-
- acpi_dev->driver_data = NULL;
-
- put_device(dev);
-}
-
-struct bus_type acpi_bus_type = {
- .name = "acpi",
- .match = acpi_bus_match,
- .probe = acpi_device_probe,
- .remove = acpi_device_remove,
- .uevent = acpi_device_uevent,
-};
-
-int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data)
-{
- return bus_for_each_dev(&acpi_bus_type, NULL, data, fn);
-}
-EXPORT_SYMBOL_GPL(acpi_bus_for_each_dev);
-
-struct acpi_dev_walk_context {
- int (*fn)(struct acpi_device *, void *);
- void *data;
-};
-
-static int acpi_dev_for_one_check(struct device *dev, void *context)
-{
- struct acpi_dev_walk_context *adwc = context;
-
- if (dev->bus != &acpi_bus_type)
- return 0;
-
- return adwc->fn(to_acpi_device(dev), adwc->data);
-}
-EXPORT_SYMBOL_GPL(acpi_dev_for_each_child);
-
-int acpi_dev_for_each_child(struct acpi_device *adev,
- int (*fn)(struct acpi_device *, void *), void *data)
-{
- struct acpi_dev_walk_context adwc = {
- .fn = fn,
- .data = data,
- };
-
- return device_for_each_child(&adev->dev, &adwc, acpi_dev_for_one_check);
-}
-
-int acpi_dev_for_each_child_reverse(struct acpi_device *adev,
- int (*fn)(struct acpi_device *, void *),
- void *data)
-{
- struct acpi_dev_walk_context adwc = {
- .fn = fn,
- .data = data,
- };
-
- return device_for_each_child_reverse(&adev->dev, &adwc, acpi_dev_for_one_check);
-}
-
/* --------------------------------------------------------------------------
Initialization/Cleanup
-------------------------------------------------------------------------- */
--
2.35.1

2022-08-25 17:42:42

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 5/5] ACPI: bus: Use the matching table, if ACPI driver has it

On Thu, Aug 25, 2022 at 8:05 PM Rafael J. Wysocki <[email protected]> wrote:
> On Thu, Aug 25, 2022 at 6:41 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > In case we have an ACPI driver, check its ID table for matching,
> > This allows to use some generic device property APIs in such
> > drivers.
>
> No new provisions for ACPI drivers, please.

OK! I will think about how to refactor a driver in question, so it
won't need this kind of trick. Meanwhile patches 1-3 can be applied
independently, if you have no objections.

--
With Best Regards,
Andy Shevchenko

2022-08-26 17:28:58

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 5/5] ACPI: bus: Use the matching table, if ACPI driver has it

On Thu, Aug 25, 2022 at 08:17:11PM +0300, Andy Shevchenko wrote:
> On Thu, Aug 25, 2022 at 8:05 PM Rafael J. Wysocki <[email protected]> wrote:
> > On Thu, Aug 25, 2022 at 6:41 PM Andy Shevchenko
> > <[email protected]> wrote:
> > >
> > > In case we have an ACPI driver, check its ID table for matching,
> > > This allows to use some generic device property APIs in such
> > > drivers.
> >
> > No new provisions for ACPI drivers, please.
>
> OK! I will think about how to refactor a driver in question, so it
> won't need this kind of trick. Meanwhile patches 1-3 can be applied
> independently, if you have no objections.

I see that you applied an equivalent patch to what I had here as patch 3.
Taking into account rejection of patches 4 and 5 I will send a v2 with
patch 1 and (modified due to drop of the 5) 2 for your convenience.

--
With Best Regards,
Andy Shevchenko


2022-08-27 12:45:26

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1 5/5] ACPI: bus: Use the matching table, if ACPI driver has it

On Fri, Aug 26, 2022 at 7:12 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Thu, Aug 25, 2022 at 08:17:11PM +0300, Andy Shevchenko wrote:
> > On Thu, Aug 25, 2022 at 8:05 PM Rafael J. Wysocki <[email protected]> wrote:
> > > On Thu, Aug 25, 2022 at 6:41 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > >
> > > > In case we have an ACPI driver, check its ID table for matching,
> > > > This allows to use some generic device property APIs in such
> > > > drivers.
> > >
> > > No new provisions for ACPI drivers, please.
> >
> > OK! I will think about how to refactor a driver in question, so it
> > won't need this kind of trick. Meanwhile patches 1-3 can be applied
> > independently, if you have no objections.
>
> I see that you applied an equivalent patch to what I had here as patch 3.
> Taking into account rejection of patches 4 and 5 I will send a v2 with
> patch 1 and (modified due to drop of the 5) 2 for your convenience.

Appreciated, thanks!