linux-kernel - Re: [PATCH v5 09/10] acpi/nfit: Move handler installing logic to driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 30 Jun 2023 13:00:48 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     "Wilczynski, Michal" <michal.wilczynski@...el.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        linux-acpi@...r.kernel.org, dan.j.williams@...el.com,
        vishal.l.verma@...el.com, lenb@...nel.org, dave.jiang@...el.com,
        ira.weiny@...el.com, rui.zhang@...el.com,
        linux-kernel@...r.kernel.org, nvdimm@...ts.linux.dev,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH v5 09/10] acpi/nfit: Move handler installing logic to driver

On Fri, Jun 30, 2023 at 11:55 AM Wilczynski, Michal
<michal.wilczynski@...el.com> wrote:
>
>
>
> On 6/29/2023 6:18 PM, Rafael J. Wysocki wrote:
> > On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski
> > <michal.wilczynski@...el.com> wrote:
> >> 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 <rafael.j.wysocki@...el.com>
> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@...el.com>
> >> ---
> >>  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(&adev->dev);
> >> -       __acpi_nfit_notify(&adev->dev, adev->handle, event);
> >> -       device_unlock(&adev->dev);
> > It's totally not necessary to rename the ACPI device variable here.
> >
> > Just add
> >
> > struct acpi_device *adev = data;
> >
> > to this function.
>
> Sure, is adev a preferred name for acpi_device ?

In new code, it is.

In the existing code, it depends.  If you do a one-line change, it is
better to retain the original naming (for the sake of clarity of the
change itself).  If you rearrange it completely, you may as well
change the names while at it.  And there is a spectrum in between.

>  I've seen a mix of different naming
> in drivers, some use device, adev, acpi_dev and so on. I suppose it's not a big deal, but
> it would be good to know.

Personally, I prefer adev, but this isn't a very strong preference.

Using "device" as a name of a struct acpi_device object (or a pointer
to one of these for that matter) is slightly misleading IMV, because
those things represent AML entities rather than actual hardware.