[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <81a7cbe7-068a-d9ab-74b1-b6891f5a6f10@redhat.com>
Date: Mon, 13 Jun 2022 22:50:45 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux ACPI <linux-acpi@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Henrique de Moraes Holschuh <hmh@....eng.br>,
Mark Gross <markgross@...nel.org>,
ibm-acpi-devel@...ts.sourceforge.net,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v2 12/16] platform/x86/thinkpad_acpi: Use
acpi_dev_for_each_child()
Hi,
On 6/13/22 20:30, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Instead of walking the list of children of an ACPI device directly,
> use acpi_dev_for_each_child() to carry out an action for all of
> the given ACPI device's children.
>
> This will help to eliminate the children list head from struct
> acpi_device as it is redundant and it is used in questionable ways
> in some places (in particular, locking is needed for walking the
> list pointed to it safely, but it is often missing).
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@...hat.com>
Rafael, feel free to take this upstream through the apci tree.
Regards,
Hans
> ---
>
> v1 -> v2:
> * Eliminate unnecessary branch (Andy).
>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 53 +++++++++++++++++------------------
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
> Index: linux-pm/drivers/platform/x86/thinkpad_acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/x86/thinkpad_acpi.c
> +++ linux-pm/drivers/platform/x86/thinkpad_acpi.c
> @@ -6841,6 +6841,31 @@ static const struct backlight_ops ibm_ba
>
> /* --------------------------------------------------------------------- */
>
> +static int __init tpacpi_evaluate_bcl(struct acpi_device *adev, void *not_used)
> +{
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *obj;
> + acpi_status status;
> + int rc;
> +
> + status = acpi_evaluate_object(adev->handle, "_BCL", NULL, &buffer);
> + if (ACPI_FAILURE(status))
> + return 0;
> +
> + obj = buffer.pointer;
> + if (!obj || obj->type != ACPI_TYPE_PACKAGE) {
> + acpi_handle_info(adev->handle,
> + "Unknown _BCL data, please report this to %s\n",
> + TPACPI_MAIL);
> + rc = 0;
> + } else {
> + rc = obj->package.count;
> + }
> + kfree(obj);
> +
> + return rc;
> +}
> +
> /*
> * Call _BCL method of video device. On some ThinkPads this will
> * switch the firmware to the ACPI brightness control mode.
> @@ -6848,37 +6873,13 @@ static const struct backlight_ops ibm_ba
>
> static int __init tpacpi_query_bcl_levels(acpi_handle handle)
> {
> - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> - union acpi_object *obj;
> - struct acpi_device *device, *child;
> - int rc;
> + struct acpi_device *device;
>
> device = acpi_fetch_acpi_dev(handle);
> if (!device)
> return 0;
>
> - rc = 0;
> - list_for_each_entry(child, &device->children, node) {
> - acpi_status status = acpi_evaluate_object(child->handle, "_BCL",
> - NULL, &buffer);
> - if (ACPI_FAILURE(status)) {
> - buffer.length = ACPI_ALLOCATE_BUFFER;
> - continue;
> - }
> -
> - obj = (union acpi_object *)buffer.pointer;
> - if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
> - pr_err("Unknown _BCL data, please report this to %s\n",
> - TPACPI_MAIL);
> - rc = 0;
> - } else {
> - rc = obj->package.count;
> - }
> - break;
> - }
> -
> - kfree(buffer.pointer);
> - return rc;
> + return acpi_dev_for_each_child(device, tpacpi_evaluate_bcl, NULL);
> }
>
>
>
>
>
Powered by blists - more mailing lists