Re: [PATCH v1] PNP: Release protocol device on registration failure

"Rafael J. Wysocki (Intel)" <[email protected]> Fri, 7 Aug 2026 17:31:51 +0200
Newsgroups gmane.linux.acpi.devel,gmane.linux.kernel
Message-ID <CAJZ5v0gbp-TwukgbvA=XBVTwYnePqo+SBSpUEUrrP6h4rRrc9A@mail.gmail.com>
On Fri, Aug 7, 2026 at 5:55 AM Yuho Choi <[email protected]> wrote:
>
> pnp_register_protocol() adds a protocol to the PNP list before calling
> device_register(). If device registration fails, pnp_remove_protocol() only
> removes the list entry and leaves the device-core reference acquired by
> device_initialize() held.
>
> Give static protocol devices a release callback and drop the reference on
> registration failure. Also stop PNP ACPI device enumeration when protocol
> registration fails, instead of using an unregistered protocol device as a
> parent.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yuho Choi <[email protected]>
> ---
>  drivers/pnp/core.c         | 9 ++++++++-
>  drivers/pnp/pnpacpi/core.c | 7 ++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
> index 81603327079c..fc3bff43fdd8 100644
> --- a/drivers/pnp/core.c
> +++ b/drivers/pnp/core.c
> @@ -23,6 +23,10 @@ static LIST_HEAD(pnp_protocols);
>  LIST_HEAD(pnp_global);
>  DEFINE_MUTEX(pnp_lock);
>
> +static void pnp_protocol_release(struct device *dev)
> +{
> +}
> +
>  /*
>   * ACPI or PNPBIOS should tell us about all platform devices, so we can
>   * skip some blind probes.  ISAPNP typically enumerates only plug-in ISA
> @@ -66,14 +70,17 @@ int pnp_register_protocol(struct pnp_protocol *protocol)
>
>         protocol->number = nodenum;
>         dev_set_name(&protocol->dev, "pnp%d", nodenum);
> +       protocol->dev.release = pnp_protocol_release;

Sashiko complains about adding an empty release callback here and IMV
it has a point:

https://sashiko.dev/#/patchset/20260807035453.948129-1-dbgh9129%40gmail.com

>
>         list_add_tail(&protocol->protocol_list, &pnp_protocols);
>
>         mutex_unlock(&pnp_lock);
>
>         ret = device_register(&protocol->dev);
> -       if (ret)
> +       if (ret) {
>                 pnp_remove_protocol(protocol);
> +               put_device(&protocol->dev);
> +       }
>
>         return ret;
>  }
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index fbf03ff007eb..da0ebc378696 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -298,12 +298,17 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
>  int pnpacpi_disabled __initdata;
>  static int __init pnpacpi_init(void)
>  {
> +       int ret;
> +
>         if (acpi_disabled || pnpacpi_disabled) {
>                 printk(KERN_INFO "pnp: PnP ACPI: disabled\n");
>                 return 0;
>         }
>         printk(KERN_INFO "pnp: PnP ACPI init\n");
> -       pnp_register_protocol(&pnpacpi_protocol);
> +       ret = pnp_register_protocol(&pnpacpi_protocol);
> +       if (ret)
> +               return ret;
> +
>         acpi_get_devices(NULL, pnpacpi_add_device_handler, NULL, NULL);
>         printk(KERN_INFO "pnp: PnP ACPI: found %d devices\n", num);
>         pnp_platform_devices = 1;
> --
> 2.43.0
>