From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Michal Wilczynski <michal.wilczynski@intel.com>
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
nvdimm@lists.linux.dev, rafael.j.wysocki@intel.com,
lenb@kernel.org, dan.j.williams@intel.com,
vishal.l.verma@intel.com, ira.weiny@intel.com
Subject: Re: [PATCH v2 3/6] ACPI: AC: Replace acpi_driver with platform_driver
Date: Fri, 6 Oct 2023 20:45:58 +0300 [thread overview]
Message-ID: <ZSBH1pd2cfk83QZ4@smile.fi.intel.com> (raw)
In-Reply-To: <20231006173055.2938160-4-michal.wilczynski@intel.com>
On Fri, Oct 06, 2023 at 08:30:52PM +0300, Michal Wilczynski wrote:
> 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.
...
> 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;
> };
When changing this, also makes sense just to check if the moving a member in
the data structure makes code shorter, but it's not a show stopper.
...
> - status = acpi_evaluate_integer(ac->device->handle, "_PSR", NULL,
> + status = acpi_evaluate_integer(ACPI_HANDLE(ac->dev), "_PSR", NULL,
> &ac->state);
> if (ACPI_FAILURE(status)) {
> - acpi_handle_info(ac->device->handle,
> + acpi_handle_info(ACPI_HANDLE(ac->dev),
Can we call ACPI_HANDLE() only once and cache that in a local variable and use
in all places?
...
> - 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);
Does it makes any sense now? Basically it duplicates the ACPI_COMPANION() call
as Rafael pointed out in previous version discussion.
> fallthrough;
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2023-10-06 17:46 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-06 17:30 [PATCH v2 0/6] Replace acpi_driver with platform_driver Michal Wilczynski
2023-10-06 17:30 ` [PATCH v2 1/6] ACPI: AC: Remove unnecessary checks Michal Wilczynski
2023-10-06 17:30 ` [PATCH v2 2/6] ACPI: AC: Use string_choices API instead of ternary operator Michal Wilczynski
2023-10-06 17:30 ` [PATCH v2 3/6] ACPI: AC: Replace acpi_driver with platform_driver Michal Wilczynski
2023-10-06 17:45 ` Andy Shevchenko [this message]
2023-10-06 19:47 ` Rafael J. Wysocki
2023-10-07 7:56 ` Andy Shevchenko
2023-10-07 10:41 ` Rafael J. Wysocki
2023-10-07 10:43 ` Rafael J. Wysocki
2023-10-09 8:40 ` Wilczynski, Michal
2023-10-09 12:27 ` Rafael J. Wysocki
2023-10-09 13:03 ` Wilczynski, Michal
2023-10-09 17:51 ` Rafael J. Wysocki
2023-10-06 17:30 ` [PATCH v2 4/6] ACPI: AC: Rename ACPI device from device to adev Michal Wilczynski
2023-10-06 17:30 ` [PATCH v2 5/6] ACPI: NFIT: Replace acpi_driver with platform_driver Michal Wilczynski
2023-10-17 8:51 ` Wilczynski, Michal
2023-10-17 10:55 ` Rafael J. Wysocki
2023-10-17 14:06 ` Rafael J. Wysocki
2023-10-17 18:24 ` Dan Williams
2023-10-18 15:38 ` Wilczynski, Michal
2023-10-06 17:30 ` [PATCH v2 6/6] ACPI: NFIT: Remove redundant call to to_acpi_dev() Michal Wilczynski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZSBH1pd2cfk83QZ4@smile.fi.intel.com \
--to=andriy.shevchenko@intel.com \
--cc=dan.j.williams@intel.com \
--cc=ira.weiny@intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.wilczynski@intel.com \
--cc=nvdimm@lists.linux.dev \
--cc=rafael.j.wysocki@intel.com \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.