[v1,0/3] Replace acpi_driver with platform_driver

This patchset is a continuation of efforts from [1] aiming to replace
acpi_driver with platform_driver. To ease up review effort I'm sending
miniseries per driver, with a replacement patch + various improvements
that were noticed by me, or during internal review.

This mini-series takes care of acpi_pad driver.

[1] - https://lore.kernel.org/linux-acpi/20231011083334.3987477-1-michal.wilczynski@intel.com/T/#t

Michal Wilczynski (3):
  ACPI: acpi_pad: Replace acpi_driver with platform_driver
  ACPI: acpi_pad: Use dev groups for sysfs
  ACPI: acpi_pad: Rename ACPI device from device to adev

 drivers/acpi/acpi_pad.c | 93 +++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 55 deletions(-)

On Mon, Oct 16, 2023 at 8:42 PM Michal Wilczynski
<michal.wilczynski@intel.com> wrote:
>
> The acpi_pad driver uses struct acpi_driver to register itself while it
> would be more logically consistent to use struct platform_driver for this
> purpose, because the corresponding platform device is present and the
> role of struct acpi_device is to amend the other bus types. ACPI devices
> are not meant to be used as proper representation of hardware entities,
> but to collect information on those hardware entities provided by the
> platform firmware.
>
> Use struct platform_driver for registering the acpi_pad driver.
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> ---
>  drivers/acpi/acpi_pad.c | 41 ++++++++++++++++++++++++-----------------
>  1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> index 7a453c5ff303..36e1049833a3 100644
> --- a/drivers/acpi/acpi_pad.c
> +++ b/drivers/acpi/acpi_pad.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
>  #include <linux/perf_event.h>
> +#include <linux/platform_device.h>
>  #include <asm/mwait.h>
>  #include <xen/xen.h>
>
> @@ -413,16 +414,16 @@ static void acpi_pad_handle_notify(acpi_handle handle)
>         mutex_unlock(&isolated_cpus_lock);
>  }
>
> -static void acpi_pad_notify(acpi_handle handle, u32 event,
> -       void *data)
> +static void acpi_pad_notify(acpi_handle handle, u32 event, void *data)
>  {
> -       struct acpi_device *device = data;
> +       struct device *dev = data;
> +       struct acpi_device *device = ACPI_COMPANION(dev);
>
>         switch (event) {
>         case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
>                 acpi_pad_handle_notify(handle);
>                 acpi_bus_generate_netlink_event(device->pnp.device_class,
> -                       dev_name(&device->dev), event, 0);
> +                                               dev_name(dev), event, 0);

This is the same mistake as in the analogous AC driver patch, as it
changes the binary interface between the kernel and user space.

Please leave it as is for now (at least in this patch).

>                 break;
>         default:
>                 pr_warn("Unsupported event [0x%x]\n", event);
> @@ -430,8 +431,10 @@ static void acpi_pad_notify(acpi_handle handle, u32 event,
>         }
>  }
>
> -static int acpi_pad_add(struct acpi_device *device)
> +static int acpi_pad_probe(struct platform_device *pdev)
>  {
> +       struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> +       struct device *dev = &pdev->dev;
>         acpi_status status;
>
>         strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
> @@ -441,7 +444,9 @@ static int acpi_pad_add(struct acpi_device *device)
>                 return -ENODEV;
>
>         status = acpi_install_notify_handler(device->handle,
> -               ACPI_DEVICE_NOTIFY, acpi_pad_notify, device);
> +                                            ACPI_DEVICE_NOTIFY,
> +                                            acpi_pad_notify,
> +                                            dev);

Passing the struct acpi_device pointer as the last argument here is
fine, no need to change that.

>         if (ACPI_FAILURE(status)) {
>                 acpi_pad_remove_sysfs(device);
>                 return -ENODEV;
> @@ -450,14 +455,17 @@ static int acpi_pad_add(struct acpi_device *device)
>         return 0;
>  }
>
> -static void acpi_pad_remove(struct acpi_device *device)
> +static void acpi_pad_remove(struct platform_device *pdev)
>  {
> +       struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> +
>         mutex_lock(&isolated_cpus_lock);
>         acpi_pad_idle_cpus(0);
>         mutex_unlock(&isolated_cpus_lock);
>
>         acpi_remove_notify_handler(device->handle,
> -               ACPI_DEVICE_NOTIFY, acpi_pad_notify);
> +                                  ACPI_DEVICE_NOTIFY,
> +                                  acpi_pad_notify);

The white space cleanup can be done in a separate patch (here and in
the other places).

>         acpi_pad_remove_sysfs(device);
>  }
>
> @@ -467,13 +475,12 @@ static const struct acpi_device_id pad_device_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(acpi, pad_device_ids);
>
> -static struct acpi_driver acpi_pad_driver = {
> -       .name = "processor_aggregator",
> -       .class = ACPI_PROCESSOR_AGGREGATOR_CLASS,
> -       .ids = pad_device_ids,
> -       .ops = {
> -               .add = acpi_pad_add,
> -               .remove = acpi_pad_remove,
> +static struct platform_driver acpi_pad_driver = {
> +       .probe = acpi_pad_probe,
> +       .remove_new = acpi_pad_remove,
> +       .driver = {
> +               .name = "processor_aggregator",
> +               .acpi_match_table = pad_device_ids,
>         },
>  };
>
> @@ -487,12 +494,12 @@ static int __init acpi_pad_init(void)
>         if (power_saving_mwait_eax == 0)
>                 return -EINVAL;
>
> -       return acpi_bus_register_driver(&acpi_pad_driver);
> +       return platform_driver_register(&acpi_pad_driver);
>  }
>
>  static void __exit acpi_pad_exit(void)
>  {
> -       acpi_bus_unregister_driver(&acpi_pad_driver);
> +       platform_driver_unregister(&acpi_pad_driver);
>  }
>
>  module_init(acpi_pad_init);
> --