[PATCH v1 0/4] ACPI: scan: Handle generic system devices directly

[PATCH v1 0/4] ACPI: scan: Handle generic system devices directly

[Rafael J. Wysocki]

Hi All,

As per the changelog of patch [1/4], there is a long-standing problem with ACPI
device enumeration that if the given device has a compatible ID which is one of
the generic system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
by the PNP scan handler and it will not be represented as a platform device, so
it cannot be handled by a platform driver.

Obviously, PNP0C01 and PNP0C02 need to be removed from acpi_pnp_device_ids[] to
address this problem, but it cannot be done without making any other changes
due to the way the legacy PNP system driver works (see the changelog of patch
[1/4] for details) and the approach used in patch [1/4] is to reserve the
"motherboard resources" directly in the ACPI core without creating platform
devices for "system" devices (whose _HID is either PNP0C01 or PNP0C02).  The
new code doing works along the lines of the PNP system driver to avoid
regressions.

The remaining patches in the series are simplifications that can be done on
top of the first one.

Patches [2-3/4] remove platform device creation code from Intel HID and VBTN
drivers that don't need to worry about _CID including PNP0C02 any more.

Patch [4/4] drops acpi_nonpnp_device_ids[] that was added as a workaround for
the issue at hand in the first place.

Thanks!

[PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device_ids[]

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There is a long-standing problem with ACPI device enumeration that
if the given device has a compatible ID which is one of the generic
system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
by the PNP scan handler and it will not be represented as a platform
device, so it cannot be handled by a platform driver.

Drivers have been working around this issue by "manually" creating
platform devices that they can bind to (see the Intel HID driver for
one example) or adding their device IDs to acpi_nonpnp_device_ids[].
None of the above is particularly clean though and the only reason why
the PNP0C01 and PNP0C02 device IDs are present in acpi_pnp_device_ids[]
is to allow the legacy PNP system driver to bind to those devices and
reserve their resources so they are not used going forward.

Obviously, to address this problem PNP0C01 and PNP0C02 need to be
dropped from acpi_pnp_device_ids[], but doing so without making any
other changes would be problematic because the ACPI core would then
create platform devices for the generic system resource device objects
and that would not work on all systems for two reasons.  First, the
PNP system driver explicitly avoids reserving I/O resources below the
"standard PC hardware" boundary, 0x100, to avoid conflicts in that range
(one possible case when this may happen is when the CMOS RTC driver is
involved), but the platform device creation code does not do that.
Second, there may be resource conflicts between the "system" devices and
the other devices in the system, possibly including conflicts with PCI
BARs.  Registering the PNP system driver via fs_initcall() helps to
manage those conflicts, even though it does not make them go away.
Resource conflicts during the registration of "motherboard resources"
that occur after PCI has claimed BARs are harmless as a rule and do
not need to be addressed in any specific way.

To overcome the issues mentioned above, use the observation that it
is not actually necessary to create any device objects in addition
to struct acpi_device ones in order to reserve the "system" device
resources because that can be done directly in the ACPI device
enumeration code.

Namely, modify acpi_default_enumeration() to add the given ACPI device
object to a special "system devices" list if its _HID is either PNP0C01
or PNP0C02 without creating a platform device for it.  Next, add a new
special acpi_scan_claim_resources() function that will be run via
fs_initcall() and will walk that list and reserve resources for each
device in it along the lines of what the PNP system driver does.

Having made the above changes, drop PNP0C01 and PNP0C02 from
acpi_pnp_device_ids[] which will allow platform devices to be created
for ACPI device objects whose _CID lists contain PNP0C01 or PNP0C02,
but the _HID is not in acpi_pnp_device_ids[].

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_pnp.c |    2 
 drivers/acpi/scan.c     |  115 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 111 insertions(+), 6 deletions(-)

--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -126,8 +126,6 @@ static const struct acpi_device_id acpi_
 	/* apple-gmux */
 	{"APP000B"},
 	/* system */
-	{"PNP0c02"},		/* General ID for reserving resources */
-	{"PNP0c01"},		/* memory controller */
 	/* rtc_cmos */
 	{"PNP0b00"},
 	{"PNP0b01"},
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -42,6 +42,7 @@ static LIST_HEAD(acpi_scan_handlers_list
 DEFINE_MUTEX(acpi_device_lock);
 LIST_HEAD(acpi_wakeup_device_list);
 static DEFINE_MUTEX(acpi_hp_context_lock);
+static LIST_HEAD(acpi_scan_system_dev_list);
 
 /*
  * The UART device described by the SPCR table is the only object which needs
@@ -2203,19 +2204,48 @@ static acpi_status acpi_bus_check_add_2(
 	return acpi_bus_check_add(handle, false, (struct acpi_device **)ret_p);
 }
 
+struct acpi_scan_system_dev {
+	struct list_head node;
+	struct acpi_device *adev;
+};
+
+static const char * const acpi_system_dev_ids[] = {
+	"PNP0C01", /* Memory controller */
+	"PNP0C02", /* Motherboard resource */
+	NULL
+};
+
 static void acpi_default_enumeration(struct acpi_device *device)
 {
 	/*
 	 * Do not enumerate devices with enumeration_by_parent flag set as
 	 * they will be enumerated by their respective parents.
 	 */
-	if (!device->flags.enumeration_by_parent) {
-		acpi_create_platform_device(device, NULL);
-		acpi_device_set_enumerated(device);
-	} else {
+	if (device->flags.enumeration_by_parent) {
 		blocking_notifier_call_chain(&acpi_reconfig_chain,
 					     ACPI_RECONFIG_DEVICE_ADD, device);
+		return;
 	}
+	if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {
+		struct acpi_scan_system_dev *sd;
+
+		/*
+		 * This is a generic system device, so there is no need to
+		 * create a platform device for it, but its resources need to be
+		 * reserved.  However, that needs to be done after all of the
+		 * other device objects have been processed and PCI has claimed
+		 * BARs in case there are resource conflicts.
+		 */
+		sd = kmalloc(sizeof(*sd), GFP_KERNEL);
+		if (sd) {
+			sd->adev = device;
+			list_add_tail(&sd->node, &acpi_scan_system_dev_list);
+		}
+	} else {
+		/* For a regular device object, create a platform device. */
+		acpi_create_platform_device(device, NULL);
+	}
+	acpi_device_set_enumerated(device);
 }
 
 static const struct acpi_device_id generic_device_ids[] = {
@@ -2571,6 +2601,83 @@ static void acpi_scan_postponed(void)
 	mutex_unlock(&acpi_dep_list_lock);
 }
 
+static void acpi_scan_claim_resources(struct acpi_device *adev)
+{
+	struct list_head resource_list = LIST_HEAD_INIT(resource_list);
+	struct resource_entry *rentry;
+	unsigned int count = 0;
+	const char *regionid;
+
+	if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
+		return;
+
+	regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
+	if (!regionid)
+		goto exit;
+
+	list_for_each_entry(rentry, &resource_list, node) {
+		struct resource *res = rentry->res;
+		struct resource *r;
+
+		/* Skip disabled and invalid resources. */
+		if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
+			continue;
+
+		if (res->flags & IORESOURCE_IO) {
+			/*
+			 * Follow the PNP system driver and on x86 skip I/O
+			 * resources that start below 0x100 (the "standard PC
+			 * hardware" boundary).
+			 */
+			if (IS_ENABLED(CONFIG_X86) && res->start < 0x100) {
+				dev_info(&adev->dev, "Skipped %pR\n", res);
+				continue;
+			}
+			r = request_region(res->start, resource_size(res), regionid);
+		} else if (res->flags & IORESOURCE_MEM) {
+			r = request_mem_region(res->start, resource_size(res), regionid);
+		} else {
+			continue;
+		}
+
+		if (r) {
+			r->flags &= ~IORESOURCE_BUSY;
+			dev_info(&adev->dev, "Reserved %pR\n", r);
+			count++;
+		} else {
+			dev_info(&adev->dev, "Could not reserve %pR\n", res);
+		}
+	}
+
+	if (!count)
+		kfree(regionid);
+
+exit:
+	acpi_dev_free_resource_list(&resource_list);
+}
+
+
+static int __init acpi_reserve_motherboard_resources(void)
+{
+	struct acpi_scan_system_dev *sd, *tmp;
+
+	guard(mutex)(&acpi_scan_lock);
+
+	list_for_each_entry_safe(sd, tmp, &acpi_scan_system_dev_list, node) {
+		acpi_scan_claim_resources(sd->adev);
+		list_del(&sd->node);
+		kfree(sd);
+	}
+
+	return 0;
+}
+
+/*
+ * Reserve motherboard resources after PCI claims BARs, but before PCI assigns
+ * resources for uninitialized PCI devices.
+ */
+fs_initcall(acpi_reserve_motherboard_resources);
+
 /**
  * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
  * @handle: Root of the namespace scope to scan.

Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device_ids[]

[Mario Limonciello]

On 12/15/25 7:34 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> There is a long-standing problem with ACPI device enumeration that
> if the given device has a compatible ID which is one of the generic
> system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
> by the PNP scan handler and it will not be represented as a platform
> device, so it cannot be handled by a platform driver.
> 
> Drivers have been working around this issue by "manually" creating
> platform devices that they can bind to (see the Intel HID driver for
> one example) or adding their device IDs to acpi_nonpnp_device_ids[].
> None of the above is particularly clean though and the only reason why
> the PNP0C01 and PNP0C02 device IDs are present in acpi_pnp_device_ids[]
> is to allow the legacy PNP system driver to bind to those devices and
> reserve their resources so they are not used going forward.
> 
> Obviously, to address this problem PNP0C01 and PNP0C02 need to be
> dropped from acpi_pnp_device_ids[], but doing so without making any
> other changes would be problematic because the ACPI core would then
> create platform devices for the generic system resource device objects
> and that would not work on all systems for two reasons.  First, the
> PNP system driver explicitly avoids reserving I/O resources below the
> "standard PC hardware" boundary, 0x100, to avoid conflicts in that range
> (one possible case when this may happen is when the CMOS RTC driver is
> involved), but the platform device creation code does not do that.
> Second, there may be resource conflicts between the "system" devices and
> the other devices in the system, possibly including conflicts with PCI
> BARs.  Registering the PNP system driver via fs_initcall() helps to
> manage those conflicts, even though it does not make them go away.
> Resource conflicts during the registration of "motherboard resources"
> that occur after PCI has claimed BARs are harmless as a rule and do
> not need to be addressed in any specific way.
> 
> To overcome the issues mentioned above, use the observation that it
> is not actually necessary to create any device objects in addition
> to struct acpi_device ones in order to reserve the "system" device
> resources because that can be done directly in the ACPI device
> enumeration code.
> 
> Namely, modify acpi_default_enumeration() to add the given ACPI device
> object to a special "system devices" list if its _HID is either PNP0C01
> or PNP0C02 without creating a platform device for it.  Next, add a new
> special acpi_scan_claim_resources() function that will be run via
> fs_initcall() and will walk that list and reserve resources for each
> device in it along the lines of what the PNP system driver does.
> 
> Having made the above changes, drop PNP0C01 and PNP0C02 from
> acpi_pnp_device_ids[] which will allow platform devices to be created
> for ACPI device objects whose _CID lists contain PNP0C01 or PNP0C02,
> but the _HID is not in acpi_pnp_device_ids[].
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/acpi_pnp.c |    2
>   drivers/acpi/scan.c     |  115 ++++++++++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 111 insertions(+), 6 deletions(-)
> 
> --- a/drivers/acpi/acpi_pnp.c
> +++ b/drivers/acpi/acpi_pnp.c
> @@ -126,8 +126,6 @@ static const struct acpi_device_id acpi_
>   	/* apple-gmux */
>   	{"APP000B"},
>   	/* system */

It might be a little bit confusing to have a comment for /* system */ 
still here but have dropped all the IDs that previously corresponded to it.

Maybe lose the comment too?

> -	{"PNP0c02"},		/* General ID for reserving resources */
> -	{"PNP0c01"},		/* memory controller */
>   	/* rtc_cmos */
>   	{"PNP0b00"},
>   	{"PNP0b01"},
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -42,6 +42,7 @@ static LIST_HEAD(acpi_scan_handlers_list
>   DEFINE_MUTEX(acpi_device_lock);
>   LIST_HEAD(acpi_wakeup_device_list);
>   static DEFINE_MUTEX(acpi_hp_context_lock);
> +static LIST_HEAD(acpi_scan_system_dev_list);
>   
>   /*
>    * The UART device described by the SPCR table is the only object which needs
> @@ -2203,19 +2204,48 @@ static acpi_status acpi_bus_check_add_2(
>   	return acpi_bus_check_add(handle, false, (struct acpi_device **)ret_p);
>   }
>   
> +struct acpi_scan_system_dev {
> +	struct list_head node;
> +	struct acpi_device *adev;
> +};
> +
> +static const char * const acpi_system_dev_ids[] = {
> +	"PNP0C01", /* Memory controller */
> +	"PNP0C02", /* Motherboard resource */
> +	NULL
> +};
> +
>   static void acpi_default_enumeration(struct acpi_device *device)
>   {
>   	/*
>   	 * Do not enumerate devices with enumeration_by_parent flag set as
>   	 * they will be enumerated by their respective parents.
>   	 */
> -	if (!device->flags.enumeration_by_parent) {
> -		acpi_create_platform_device(device, NULL);
> -		acpi_device_set_enumerated(device);
> -	} else {
> +	if (device->flags.enumeration_by_parent) {
>   		blocking_notifier_call_chain(&acpi_reconfig_chain,
>   					     ACPI_RECONFIG_DEVICE_ADD, device);
> +		return;
>   	}
> +	if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {
> +		struct acpi_scan_system_dev *sd;
> +
> +		/*
> +		 * This is a generic system device, so there is no need to
> +		 * create a platform device for it, but its resources need to be
> +		 * reserved.  However, that needs to be done after all of the
> +		 * other device objects have been processed and PCI has claimed
> +		 * BARs in case there are resource conflicts.
> +		 */
> +		sd = kmalloc(sizeof(*sd), GFP_KERNEL);
> +		if (sd) {
> +			sd->adev = device;
> +			list_add_tail(&sd->node, &acpi_scan_system_dev_list);
> +		}
> +	} else {
> +		/* For a regular device object, create a platform device. */
> +		acpi_create_platform_device(device, NULL);
> +	}
> +	acpi_device_set_enumerated(device);
>   }
>   
>   static const struct acpi_device_id generic_device_ids[] = {
> @@ -2571,6 +2601,83 @@ static void acpi_scan_postponed(void)
>   	mutex_unlock(&acpi_dep_list_lock);
>   }
>   
> +static void acpi_scan_claim_resources(struct acpi_device *adev)
> +{
> +	struct list_head resource_list = LIST_HEAD_INIT(resource_list);
> +	struct resource_entry *rentry;
> +	unsigned int count = 0;
> +	const char *regionid;
> +
> +	if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
> +		return;
> +
> +	regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
> +	if (!regionid)
> +		goto exit;
> +
> +	list_for_each_entry(rentry, &resource_list, node) {
> +		struct resource *res = rentry->res;
> +		struct resource *r;
> +
> +		/* Skip disabled and invalid resources. */
> +		if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
> +			continue;
> +
> +		if (res->flags & IORESOURCE_IO) {
> +			/*
> +			 * Follow the PNP system driver and on x86 skip I/O
> +			 * resources that start below 0x100 (the "standard PC
> +			 * hardware" boundary).
> +			 */
> +			if (IS_ENABLED(CONFIG_X86) && res->start < 0x100) {
> +				dev_info(&adev->dev, "Skipped %pR\n", res);
> +				continue;
> +			}
> +			r = request_region(res->start, resource_size(res), regionid);
> +		} else if (res->flags & IORESOURCE_MEM) {
> +			r = request_mem_region(res->start, resource_size(res), regionid);
> +		} else {
> +			continue;
> +		}
> +
> +		if (r) {
> +			r->flags &= ~IORESOURCE_BUSY;
> +			dev_info(&adev->dev, "Reserved %pR\n", r);
> +			count++;
> +		} else {
> +			dev_info(&adev->dev, "Could not reserve %pR\n", res);

Shouldn't this be louder?  Like warn?  Or do you think there will be 
normal conditions we see this happening?

> +		}
> +	}
> +
> +	if (!count)
> +		kfree(regionid);
> +
> +exit:
> +	acpi_dev_free_resource_list(&resource_list);
> +}
> +
> +
> +static int __init acpi_reserve_motherboard_resources(void)
> +{
> +	struct acpi_scan_system_dev *sd, *tmp;
> +
> +	guard(mutex)(&acpi_scan_lock);
> +
> +	list_for_each_entry_safe(sd, tmp, &acpi_scan_system_dev_list, node) {
> +		acpi_scan_claim_resources(sd->adev);
> +		list_del(&sd->node);
> +		kfree(sd);
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Reserve motherboard resources after PCI claims BARs, but before PCI assigns
> + * resources for uninitialized PCI devices.
> + */
> +fs_initcall(acpi_reserve_motherboard_resources);
> +
>   /**
>    * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
>    * @handle: Root of the namespace scope to scan.
> 
> 
> 
> 

Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device_ids[]

[Rafael J. Wysocki]

On Mon, Dec 15, 2025 at 10:18 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> On 12/15/25 7:34 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > There is a long-standing problem with ACPI device enumeration that
> > if the given device has a compatible ID which is one of the generic
> > system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
> > by the PNP scan handler and it will not be represented as a platform
> > device, so it cannot be handled by a platform driver.
> >
> > Drivers have been working around this issue by "manually" creating
> > platform devices that they can bind to (see the Intel HID driver for
> > one example) or adding their device IDs to acpi_nonpnp_device_ids[].
> > None of the above is particularly clean though and the only reason why
> > the PNP0C01 and PNP0C02 device IDs are present in acpi_pnp_device_ids[]
> > is to allow the legacy PNP system driver to bind to those devices and
> > reserve their resources so they are not used going forward.
> >
> > Obviously, to address this problem PNP0C01 and PNP0C02 need to be
> > dropped from acpi_pnp_device_ids[], but doing so without making any
> > other changes would be problematic because the ACPI core would then
> > create platform devices for the generic system resource device objects
> > and that would not work on all systems for two reasons.  First, the
> > PNP system driver explicitly avoids reserving I/O resources below the
> > "standard PC hardware" boundary, 0x100, to avoid conflicts in that range
> > (one possible case when this may happen is when the CMOS RTC driver is
> > involved), but the platform device creation code does not do that.
> > Second, there may be resource conflicts between the "system" devices and
> > the other devices in the system, possibly including conflicts with PCI
> > BARs.  Registering the PNP system driver via fs_initcall() helps to
> > manage those conflicts, even though it does not make them go away.
> > Resource conflicts during the registration of "motherboard resources"
> > that occur after PCI has claimed BARs are harmless as a rule and do
> > not need to be addressed in any specific way.
> >
> > To overcome the issues mentioned above, use the observation that it
> > is not actually necessary to create any device objects in addition
> > to struct acpi_device ones in order to reserve the "system" device
> > resources because that can be done directly in the ACPI device
> > enumeration code.
> >
> > Namely, modify acpi_default_enumeration() to add the given ACPI device
> > object to a special "system devices" list if its _HID is either PNP0C01
> > or PNP0C02 without creating a platform device for it.  Next, add a new
> > special acpi_scan_claim_resources() function that will be run via
> > fs_initcall() and will walk that list and reserve resources for each
> > device in it along the lines of what the PNP system driver does.
> >
> > Having made the above changes, drop PNP0C01 and PNP0C02 from
> > acpi_pnp_device_ids[] which will allow platform devices to be created
> > for ACPI device objects whose _CID lists contain PNP0C01 or PNP0C02,
> > but the _HID is not in acpi_pnp_device_ids[].
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/acpi/acpi_pnp.c |    2
> >   drivers/acpi/scan.c     |  115 ++++++++++++++++++++++++++++++++++++++++++++++--
> >   2 files changed, 111 insertions(+), 6 deletions(-)
> >
> > --- a/drivers/acpi/acpi_pnp.c
> > +++ b/drivers/acpi/acpi_pnp.c
> > @@ -126,8 +126,6 @@ static const struct acpi_device_id acpi_
> >       /* apple-gmux */
> >       {"APP000B"},
> >       /* system */
>
> It might be a little bit confusing to have a comment for /* system */
> still here but have dropped all the IDs that previously corresponded to it.
>
> Maybe lose the comment too?

Sure, missed that.  I'll remove it when applying the patch.

> > -     {"PNP0c02"},            /* General ID for reserving resources */
> > -     {"PNP0c01"},            /* memory controller */
> >       /* rtc_cmos */
> >       {"PNP0b00"},
> >       {"PNP0b01"},
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -42,6 +42,7 @@ static LIST_HEAD(acpi_scan_handlers_list
> >   DEFINE_MUTEX(acpi_device_lock);
> >   LIST_HEAD(acpi_wakeup_device_list);
> >   static DEFINE_MUTEX(acpi_hp_context_lock);
> > +static LIST_HEAD(acpi_scan_system_dev_list);
> >
> >   /*
> >    * The UART device described by the SPCR table is the only object which needs
> > @@ -2203,19 +2204,48 @@ static acpi_status acpi_bus_check_add_2(
> >       return acpi_bus_check_add(handle, false, (struct acpi_device **)ret_p);
> >   }
> >
> > +struct acpi_scan_system_dev {
> > +     struct list_head node;
> > +     struct acpi_device *adev;
> > +};
> > +
> > +static const char * const acpi_system_dev_ids[] = {
> > +     "PNP0C01", /* Memory controller */
> > +     "PNP0C02", /* Motherboard resource */
> > +     NULL
> > +};
> > +
> >   static void acpi_default_enumeration(struct acpi_device *device)
> >   {
> >       /*
> >        * Do not enumerate devices with enumeration_by_parent flag set as
> >        * they will be enumerated by their respective parents.
> >        */
> > -     if (!device->flags.enumeration_by_parent) {
> > -             acpi_create_platform_device(device, NULL);
> > -             acpi_device_set_enumerated(device);
> > -     } else {
> > +     if (device->flags.enumeration_by_parent) {
> >               blocking_notifier_call_chain(&acpi_reconfig_chain,
> >                                            ACPI_RECONFIG_DEVICE_ADD, device);
> > +             return;
> >       }
> > +     if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {
> > +             struct acpi_scan_system_dev *sd;
> > +
> > +             /*
> > +              * This is a generic system device, so there is no need to
> > +              * create a platform device for it, but its resources need to be
> > +              * reserved.  However, that needs to be done after all of the
> > +              * other device objects have been processed and PCI has claimed
> > +              * BARs in case there are resource conflicts.
> > +              */
> > +             sd = kmalloc(sizeof(*sd), GFP_KERNEL);
> > +             if (sd) {
> > +                     sd->adev = device;
> > +                     list_add_tail(&sd->node, &acpi_scan_system_dev_list);
> > +             }
> > +     } else {
> > +             /* For a regular device object, create a platform device. */
> > +             acpi_create_platform_device(device, NULL);
> > +     }
> > +     acpi_device_set_enumerated(device);
> >   }
> >
> >   static const struct acpi_device_id generic_device_ids[] = {
> > @@ -2571,6 +2601,83 @@ static void acpi_scan_postponed(void)
> >       mutex_unlock(&acpi_dep_list_lock);
> >   }
> >
> > +static void acpi_scan_claim_resources(struct acpi_device *adev)
> > +{
> > +     struct list_head resource_list = LIST_HEAD_INIT(resource_list);
> > +     struct resource_entry *rentry;
> > +     unsigned int count = 0;
> > +     const char *regionid;
> > +
> > +     if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
> > +             return;
> > +
> > +     regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
> > +     if (!regionid)
> > +             goto exit;
> > +
> > +     list_for_each_entry(rentry, &resource_list, node) {
> > +             struct resource *res = rentry->res;
> > +             struct resource *r;
> > +
> > +             /* Skip disabled and invalid resources. */
> > +             if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
> > +                     continue;
> > +
> > +             if (res->flags & IORESOURCE_IO) {
> > +                     /*
> > +                      * Follow the PNP system driver and on x86 skip I/O
> > +                      * resources that start below 0x100 (the "standard PC
> > +                      * hardware" boundary).
> > +                      */
> > +                     if (IS_ENABLED(CONFIG_X86) && res->start < 0x100) {
> > +                             dev_info(&adev->dev, "Skipped %pR\n", res);
> > +                             continue;
> > +                     }
> > +                     r = request_region(res->start, resource_size(res), regionid);
> > +             } else if (res->flags & IORESOURCE_MEM) {
> > +                     r = request_mem_region(res->start, resource_size(res), regionid);
> > +             } else {
> > +                     continue;
> > +             }
> > +
> > +             if (r) {
> > +                     r->flags &= ~IORESOURCE_BUSY;
> > +                     dev_info(&adev->dev, "Reserved %pR\n", r);
> > +                     count++;
> > +             } else {
> > +                     dev_info(&adev->dev, "Could not reserve %pR\n", res);
>
> Shouldn't this be louder?  Like warn?  Or do you think there will be
> normal conditions we see this happening?

This happens on all systems I have, so ...

Also the PNP system driver doesn't make a fuss about this, but I may
as well copy the relevant comment from there to here.

Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device_ids[]

[Andy Shevchenko]

On Mon, Dec 15, 2025 at 02:34:06PM +0100, Rafael J. Wysocki wrote:

> There is a long-standing problem with ACPI device enumeration that
> if the given device has a compatible ID which is one of the generic
> system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
> by the PNP scan handler and it will not be represented as a platform
> device, so it cannot be handled by a platform driver.
> 
> Drivers have been working around this issue by "manually" creating
> platform devices that they can bind to (see the Intel HID driver for
> one example) or adding their device IDs to acpi_nonpnp_device_ids[].
> None of the above is particularly clean though and the only reason why
> the PNP0C01 and PNP0C02 device IDs are present in acpi_pnp_device_ids[]
> is to allow the legacy PNP system driver to bind to those devices and
> reserve their resources so they are not used going forward.
> 
> Obviously, to address this problem PNP0C01 and PNP0C02 need to be
> dropped from acpi_pnp_device_ids[], but doing so without making any
> other changes would be problematic because the ACPI core would then
> create platform devices for the generic system resource device objects
> and that would not work on all systems for two reasons.  First, the
> PNP system driver explicitly avoids reserving I/O resources below the
> "standard PC hardware" boundary, 0x100, to avoid conflicts in that range
> (one possible case when this may happen is when the CMOS RTC driver is
> involved), but the platform device creation code does not do that.
> Second, there may be resource conflicts between the "system" devices and
> the other devices in the system, possibly including conflicts with PCI
> BARs.  Registering the PNP system driver via fs_initcall() helps to
> manage those conflicts, even though it does not make them go away.
> Resource conflicts during the registration of "motherboard resources"
> that occur after PCI has claimed BARs are harmless as a rule and do
> not need to be addressed in any specific way.
> 
> To overcome the issues mentioned above, use the observation that it
> is not actually necessary to create any device objects in addition
> to struct acpi_device ones in order to reserve the "system" device
> resources because that can be done directly in the ACPI device
> enumeration code.
> 
> Namely, modify acpi_default_enumeration() to add the given ACPI device
> object to a special "system devices" list if its _HID is either PNP0C01
> or PNP0C02 without creating a platform device for it.  Next, add a new
> special acpi_scan_claim_resources() function that will be run via
> fs_initcall() and will walk that list and reserve resources for each
> device in it along the lines of what the PNP system driver does.
> 
> Having made the above changes, drop PNP0C01 and PNP0C02 from
> acpi_pnp_device_ids[] which will allow platform devices to be created
> for ACPI device objects whose _CID lists contain PNP0C01 or PNP0C02,
> but the _HID is not in acpi_pnp_device_ids[].

...

> +static const char * const acpi_system_dev_ids[] = {
> +	"PNP0C01", /* Memory controller */
> +	"PNP0C02", /* Motherboard resource */
> +	NULL
> +};

...

> +	if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {

Using -1 makes sense when we have no direct visibility of the mentioned array.
Here we have it visible and statically defined, hence the ARRAY_SIZE() is more
appropriate.

> +		struct acpi_scan_system_dev *sd;
> +
> +		/*
> +		 * This is a generic system device, so there is no need to
> +		 * create a platform device for it, but its resources need to be
> +		 * reserved.  However, that needs to be done after all of the
> +		 * other device objects have been processed and PCI has claimed
> +		 * BARs in case there are resource conflicts.
> +		 */
> +		sd = kmalloc(sizeof(*sd), GFP_KERNEL);
> +		if (sd) {
> +			sd->adev = device;
> +			list_add_tail(&sd->node, &acpi_scan_system_dev_list);
> +		}
> +	} else {
> +		/* For a regular device object, create a platform device. */
> +		acpi_create_platform_device(device, NULL);
> +	}
> +	acpi_device_set_enumerated(device);
>  }

...

> +static void acpi_scan_claim_resources(struct acpi_device *adev)
> +{
> +	struct list_head resource_list = LIST_HEAD_INIT(resource_list);
> +	struct resource_entry *rentry;
> +	unsigned int count = 0;
> +	const char *regionid;

> +	if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
> +		return;

Strictly speaking the acpi_dev_free_resource_list() still needs to be called
on 0 return as it's called only for the error cases.

I think this is the first and the only time I see a combined comparison <= 0
for the acpi_dev_get_resources().

> +	regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
> +	if (!regionid)
> +		goto exit;
> +
> +	list_for_each_entry(rentry, &resource_list, node) {
> +		struct resource *res = rentry->res;
> +		struct resource *r;
> +
> +		/* Skip disabled and invalid resources. */
> +		if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
> +			continue;

> +		if (res->flags & IORESOURCE_IO) {

We have resource_type() helper.

And I believe the direct comparison in this case is better.

> +			/*
> +			 * Follow the PNP system driver and on x86 skip I/O
> +			 * resources that start below 0x100 (the "standard PC
> +			 * hardware" boundary).
> +			 */
> +			if (IS_ENABLED(CONFIG_X86) && res->start < 0x100) {
> +				dev_info(&adev->dev, "Skipped %pR\n", res);
> +				continue;
> +			}
> +			r = request_region(res->start, resource_size(res), regionid);
> +		} else if (res->flags & IORESOURCE_MEM) {
> +			r = request_mem_region(res->start, resource_size(res), regionid);
> +		} else {
> +			continue;
> +		}
> +
> +		if (r) {
> +			r->flags &= ~IORESOURCE_BUSY;
> +			dev_info(&adev->dev, "Reserved %pR\n", r);
> +			count++;
> +		} else {
> +			dev_info(&adev->dev, "Could not reserve %pR\n", res);
> +		}
> +	}
> +
> +	if (!count)
> +		kfree(regionid);
> +
> +exit:
> +	acpi_dev_free_resource_list(&resource_list);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device_ids[]

[Rafael J. Wysocki]

On Sun, Dec 28, 2025 at 8:11 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Dec 15, 2025 at 02:34:06PM +0100, Rafael J. Wysocki wrote:
>
> > There is a long-standing problem with ACPI device enumeration that
> > if the given device has a compatible ID which is one of the generic
> > system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
> > by the PNP scan handler and it will not be represented as a platform
> > device, so it cannot be handled by a platform driver.
> >
> > Drivers have been working around this issue by "manually" creating
> > platform devices that they can bind to (see the Intel HID driver for
> > one example) or adding their device IDs to acpi_nonpnp_device_ids[].
> > None of the above is particularly clean though and the only reason why
> > the PNP0C01 and PNP0C02 device IDs are present in acpi_pnp_device_ids[]
> > is to allow the legacy PNP system driver to bind to those devices and
> > reserve their resources so they are not used going forward.
> >
> > Obviously, to address this problem PNP0C01 and PNP0C02 need to be
> > dropped from acpi_pnp_device_ids[], but doing so without making any
> > other changes would be problematic because the ACPI core would then
> > create platform devices for the generic system resource device objects
> > and that would not work on all systems for two reasons.  First, the
> > PNP system driver explicitly avoids reserving I/O resources below the
> > "standard PC hardware" boundary, 0x100, to avoid conflicts in that range
> > (one possible case when this may happen is when the CMOS RTC driver is
> > involved), but the platform device creation code does not do that.
> > Second, there may be resource conflicts between the "system" devices and
> > the other devices in the system, possibly including conflicts with PCI
> > BARs.  Registering the PNP system driver via fs_initcall() helps to
> > manage those conflicts, even though it does not make them go away.
> > Resource conflicts during the registration of "motherboard resources"
> > that occur after PCI has claimed BARs are harmless as a rule and do
> > not need to be addressed in any specific way.
> >
> > To overcome the issues mentioned above, use the observation that it
> > is not actually necessary to create any device objects in addition
> > to struct acpi_device ones in order to reserve the "system" device
> > resources because that can be done directly in the ACPI device
> > enumeration code.
> >
> > Namely, modify acpi_default_enumeration() to add the given ACPI device
> > object to a special "system devices" list if its _HID is either PNP0C01
> > or PNP0C02 without creating a platform device for it.  Next, add a new
> > special acpi_scan_claim_resources() function that will be run via
> > fs_initcall() and will walk that list and reserve resources for each
> > device in it along the lines of what the PNP system driver does.
> >
> > Having made the above changes, drop PNP0C01 and PNP0C02 from
> > acpi_pnp_device_ids[] which will allow platform devices to be created
> > for ACPI device objects whose _CID lists contain PNP0C01 or PNP0C02,
> > but the _HID is not in acpi_pnp_device_ids[].
>
> ...
>
> > +static const char * const acpi_system_dev_ids[] = {
> > +     "PNP0C01", /* Memory controller */
> > +     "PNP0C02", /* Motherboard resource */
> > +     NULL
> > +};
>
> ...
>
> > +     if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {
>
> Using -1 makes sense when we have no direct visibility of the mentioned array.
> Here we have it visible and statically defined, hence the ARRAY_SIZE() is more
> appropriate.

But as it stands it is more consistent with what happens elsewhere in this file.

> > +             struct acpi_scan_system_dev *sd;
> > +
> > +             /*
> > +              * This is a generic system device, so there is no need to
> > +              * create a platform device for it, but its resources need to be
> > +              * reserved.  However, that needs to be done after all of the
> > +              * other device objects have been processed and PCI has claimed
> > +              * BARs in case there are resource conflicts.
> > +              */
> > +             sd = kmalloc(sizeof(*sd), GFP_KERNEL);
> > +             if (sd) {
> > +                     sd->adev = device;
> > +                     list_add_tail(&sd->node, &acpi_scan_system_dev_list);
> > +             }
> > +     } else {
> > +             /* For a regular device object, create a platform device. */
> > +             acpi_create_platform_device(device, NULL);
> > +     }
> > +     acpi_device_set_enumerated(device);
> >  }
>
> ...
>
> > +static void acpi_scan_claim_resources(struct acpi_device *adev)
> > +{
> > +     struct list_head resource_list = LIST_HEAD_INIT(resource_list);
> > +     struct resource_entry *rentry;
> > +     unsigned int count = 0;
> > +     const char *regionid;
>
> > +     if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
> > +             return;
>
> Strictly speaking the acpi_dev_free_resource_list() still needs to be called
> on 0 return as it's called only for the error cases.

The return value of 0 means that the list is empty, so there's nothing to free.

> I think this is the first and the only time I see a combined comparison <= 0
> for the acpi_dev_get_resources().

I don't see a problem here though.

> > +     regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
> > +     if (!regionid)
> > +             goto exit;
> > +
> > +     list_for_each_entry(rentry, &resource_list, node) {
> > +             struct resource *res = rentry->res;
> > +             struct resource *r;
> > +
> > +             /* Skip disabled and invalid resources. */
> > +             if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
> > +                     continue;
>
> > +             if (res->flags & IORESOURCE_IO) {
>
> We have resource_type() helper.

I had considered using it, but then I couldn't convince myself that it
would matter, so I went for simpler code.

> And I believe the direct comparison in this case is better.

You seem to be arguing that it matters after all, so why does it matter?

Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device_ids[]

[Andy Shevchenko]

On Mon, Dec 29, 2025 at 02:12:10PM +0100, Rafael J. Wysocki wrote:
> On Sun, Dec 28, 2025 at 8:11 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Dec 15, 2025 at 02:34:06PM +0100, Rafael J. Wysocki wrote:

...

> > > +     if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {
> >
> > Using -1 makes sense when we have no direct visibility of the mentioned array.
> > Here we have it visible and statically defined, hence the ARRAY_SIZE() is more
> > appropriate.
> 
> But as it stands it is more consistent with what happens elsewhere in this file.

It's not a big deal, especially if there is some consistency involved.

...

> > > +static void acpi_scan_claim_resources(struct acpi_device *adev)
> > > +{
> > > +     struct list_head resource_list = LIST_HEAD_INIT(resource_list);
> > > +     struct resource_entry *rentry;
> > > +     unsigned int count = 0;
> > > +     const char *regionid;
> >
> > > +     if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
> > > +             return;
> >
> > Strictly speaking the acpi_dev_free_resource_list() still needs to be called
> > on 0 return as it's called only for the error cases.
> 
> The return value of 0 means that the list is empty, so there's nothing to free.
> 
> > I think this is the first and the only time I see a combined comparison <= 0
> > for the acpi_dev_get_resources().
> 
> I don't see a problem here though.

There is no problem, the only minor thing is that the pattern I see seems only
here.

> > > +     regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
> > > +     if (!regionid)
> > > +             goto exit;
> > > +
> > > +     list_for_each_entry(rentry, &resource_list, node) {
> > > +             struct resource *res = rentry->res;
> > > +             struct resource *r;
> > > +
> > > +             /* Skip disabled and invalid resources. */
> > > +             if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
> > > +                     continue;
> >
> > > +             if (res->flags & IORESOURCE_IO) {
> >
> > We have resource_type() helper.
> 
> I had considered using it, but then I couldn't convince myself that it
> would matter, so I went for simpler code.
> 
> > And I believe the direct comparison in this case is better.
> 
> You seem to be arguing that it matters after all, so why does it matter?

In the light of the resent findings of the resource_size() use on zeroed data
it also might be that in some cases the opposite happens, i.e. the resource had
initially a more bits of type assigned while checked for the certain one(s).
In such case the resource_type() == IORESOURCE_foo will fail on those.

I assume, nevertheless, that the code in question is quite unlikely exposed to
such mistakes.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 2/4] platform/x86/intel/hid: Stop creating a platform device

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Now that "system" devices are represented as platform devices, they
are not claimed by the PNP ACPI scan handler any more and the Intel
HID platform devices should be created by the ACPI core, so the
driver does not need to attempt to create a platform device by
itself.

Accordingly, make it stop doing so.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/intel/hid.c |   41 ---------------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

--- a/drivers/platform/x86/intel/hid.c
+++ b/drivers/platform/x86/intel/hid.c
@@ -767,43 +767,4 @@ static struct platform_driver intel_hid_
 	.remove = intel_hid_remove,
 };
 
-/*
- * Unfortunately, some laptops provide a _HID="INT33D5" device with
- * _CID="PNP0C02".  This causes the pnpacpi scan driver to claim the
- * ACPI node, so no platform device will be created.  The pnpacpi
- * driver rejects this device in subsequent processing, so no physical
- * node is created at all.
- *
- * As a workaround until the ACPI core figures out how to handle
- * this corner case, manually ask the ACPI platform device code to
- * claim the ACPI node.
- */
-static acpi_status __init
-check_acpi_dev(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	const struct acpi_device_id *ids = context;
-	struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
-
-	if (dev && acpi_match_device_ids(dev, ids) == 0)
-		if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL)))
-			dev_info(&dev->dev,
-				 "intel-hid: created platform device\n");
-
-	return AE_OK;
-}
-
-static int __init intel_hid_init(void)
-{
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX, check_acpi_dev, NULL,
-			    (void *)intel_hid_ids, NULL);
-
-	return platform_driver_register(&intel_hid_pl_driver);
-}
-module_init(intel_hid_init);
-
-static void __exit intel_hid_exit(void)
-{
-	platform_driver_unregister(&intel_hid_pl_driver);
-}
-module_exit(intel_hid_exit);
+module_platform_driver(intel_hid_pl_driver);

Re: [PATCH v1 2/4] platform/x86/intel/hid: Stop creating a platform device

[Andy Shevchenko]

On Mon, Dec 15, 2025 at 02:35:05PM +0100, Rafael J. Wysocki wrote:
> 
> Now that "system" devices are represented as platform devices, they
> are not claimed by the PNP ACPI scan handler any more and the Intel
> HID platform devices should be created by the ACPI core, so the
> driver does not need to attempt to create a platform device by
> itself.
> 
> Accordingly, make it stop doing so.
> 
> No intentional functional impact.

...

>  	.remove = intel_hid_remove,
>  };

>  

This blank line now can also be removed as the module_platform_driver() coupled
with the above structure initialiser.

> -/*
> - * Unfortunately, some laptops provide a _HID="INT33D5" device with
> - * _CID="PNP0C02".  This causes the pnpacpi scan driver to claim the
> - * ACPI node, so no platform device will be created.  The pnpacpi
> - * driver rejects this device in subsequent processing, so no physical
> - * node is created at all.
> - *
> - * As a workaround until the ACPI core figures out how to handle
> - * this corner case, manually ask the ACPI platform device code to
> - * claim the ACPI node.
> - */
> -static acpi_status __init
> -check_acpi_dev(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> -	const struct acpi_device_id *ids = context;
> -	struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
> -
> -	if (dev && acpi_match_device_ids(dev, ids) == 0)
> -		if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL)))
> -			dev_info(&dev->dev,
> -				 "intel-hid: created platform device\n");
> -
> -	return AE_OK;
> -}
> -
> -static int __init intel_hid_init(void)
> -{
> -	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> -			    ACPI_UINT32_MAX, check_acpi_dev, NULL,
> -			    (void *)intel_hid_ids, NULL);
> -
> -	return platform_driver_register(&intel_hid_pl_driver);
> -}
> -module_init(intel_hid_init);
> -
> -static void __exit intel_hid_exit(void)
> -{
> -	platform_driver_unregister(&intel_hid_pl_driver);
> -}
> -module_exit(intel_hid_exit);
> +module_platform_driver(intel_hid_pl_driver);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/4] platform/x86/intel/hid: Stop creating a platform device

[Rafael J. Wysocki]

On Sun, Dec 28, 2025 at 8:13 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Dec 15, 2025 at 02:35:05PM +0100, Rafael J. Wysocki wrote:
> >
> > Now that "system" devices are represented as platform devices, they
> > are not claimed by the PNP ACPI scan handler any more and the Intel
> > HID platform devices should be created by the ACPI core, so the
> > driver does not need to attempt to create a platform device by
> > itself.
> >
> > Accordingly, make it stop doing so.
> >
> > No intentional functional impact.
>
> ...
>
> >       .remove = intel_hid_remove,
> >  };
>
> >
>
> This blank line now can also be removed as the module_platform_driver() coupled
> with the above structure initialiser.
>
> > -/*
> > - * Unfortunately, some laptops provide a _HID="INT33D5" device with
> > - * _CID="PNP0C02".  This causes the pnpacpi scan driver to claim the
> > - * ACPI node, so no platform device will be created.  The pnpacpi
> > - * driver rejects this device in subsequent processing, so no physical
> > - * node is created at all.
> > - *
> > - * As a workaround until the ACPI core figures out how to handle
> > - * this corner case, manually ask the ACPI platform device code to
> > - * claim the ACPI node.
> > - */
> > -static acpi_status __init
> > -check_acpi_dev(acpi_handle handle, u32 lvl, void *context, void **rv)
> > -{
> > -     const struct acpi_device_id *ids = context;
> > -     struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
> > -
> > -     if (dev && acpi_match_device_ids(dev, ids) == 0)
> > -             if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL)))
> > -                     dev_info(&dev->dev,
> > -                              "intel-hid: created platform device\n");
> > -
> > -     return AE_OK;
> > -}
> > -
> > -static int __init intel_hid_init(void)
> > -{
> > -     acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> > -                         ACPI_UINT32_MAX, check_acpi_dev, NULL,
> > -                         (void *)intel_hid_ids, NULL);
> > -
> > -     return platform_driver_register(&intel_hid_pl_driver);
> > -}
> > -module_init(intel_hid_init);
> > -
> > -static void __exit intel_hid_exit(void)
> > -{
> > -     platform_driver_unregister(&intel_hid_pl_driver);
> > -}
> > -module_exit(intel_hid_exit);
> > +module_platform_driver(intel_hid_pl_driver);

Feel free to send a patch to remove it then, perhaps along with the
one in the other driver.

Re: [PATCH v1 2/4] platform/x86/intel/hid: Stop creating a platform device

[Andy Shevchenko]

On Mon, Dec 29, 2025 at 02:07:23PM +0100, Rafael J. Wysocki wrote:
> On Sun, Dec 28, 2025 at 8:13 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Dec 15, 2025 at 02:35:05PM +0100, Rafael J. Wysocki wrote:

...

> > >       .remove = intel_hid_remove,
> > >  };
> >
> > This blank line now can also be removed as the module_platform_driver() coupled
> > with the above structure initialiser.

> Feel free to send a patch to remove it then, perhaps along with the
> one in the other driver.

If it's already applied the mention change would be just a churn, no need to
bother with it.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 3/4] platform/x86/intel/vbtn: Stop creating a platform device

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Now that "system" devices are represented as platform devices, they
are not claimed by the PNP ACPI scan handler any more and the Intel
virtual button array platform devices should be created by the ACPI
core, so the driver does not need to attempt to create a platform
device by itself.

Accordingly, make it stop doing so.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/intel/vbtn.c |   30 +-----------------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

--- a/drivers/platform/x86/intel/vbtn.c
+++ b/drivers/platform/x86/intel/vbtn.c
@@ -390,32 +390,4 @@ static struct platform_driver intel_vbtn
 	.remove = intel_vbtn_remove,
 };
 
-static acpi_status __init
-check_acpi_dev(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	const struct acpi_device_id *ids = context;
-	struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
-
-	if (dev && acpi_match_device_ids(dev, ids) == 0)
-		if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL)))
-			dev_info(&dev->dev,
-				 "intel-vbtn: created platform device\n");
-
-	return AE_OK;
-}
-
-static int __init intel_vbtn_init(void)
-{
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX, check_acpi_dev, NULL,
-			    (void *)intel_vbtn_ids, NULL);
-
-	return platform_driver_register(&intel_vbtn_pl_driver);
-}
-module_init(intel_vbtn_init);
-
-static void __exit intel_vbtn_exit(void)
-{
-	platform_driver_unregister(&intel_vbtn_pl_driver);
-}
-module_exit(intel_vbtn_exit);
+module_platform_driver(intel_vbtn_pl_driver);

Re: [PATCH v1 3/4] platform/x86/intel/vbtn: Stop creating a platform device

[Andy Shevchenko]

On Mon, Dec 15, 2025 at 02:35:44PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Now that "system" devices are represented as platform devices, they
> are not claimed by the PNP ACPI scan handler any more and the Intel
> virtual button array platform devices should be created by the ACPI
> core, so the driver does not need to attempt to create a platform
> device by itself.
> 
> Accordingly, make it stop doing so.
> 
> No intentional functional impact.

...

>  	.remove = intel_vbtn_remove,
>  };
>  

Same comment about this blank line.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 4/4] ACPI: PNP: Drop acpi_nonpnp_device_ids[]

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Now that "system" devices are represented as platform devices, they
are not claimed by the PNP ACPI scan handler any more and platform
devices can be created for ACPI device objects listing "system"
device IDs as their compatible device IDs.

Accordingly, it should not be necessary any more to add device IDs
to acpi_nonpnp_device_ids[], so drop it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_pnp.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -344,24 +344,10 @@ static bool acpi_pnp_match(const char *i
 	return false;
 }
 
-/*
- * If one of the device IDs below is present in the list of device IDs of a
- * given ACPI device object, the PNP scan handler will not attach to that
- * object, because there is a proper non-PNP driver in the kernel for the
- * device represented by it.
- */
-static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
-	{"INT3F0D"},
-	{"INTC1080"},
-	{"INTC1081"},
-	{"INTC1099"},
-	{""},
-};
-
 static int acpi_pnp_attach(struct acpi_device *adev,
 			   const struct acpi_device_id *id)
 {
-	return !!acpi_match_device_ids(adev, acpi_nonpnp_device_ids);
+	return true;
 }
 
 static struct acpi_scan_handler acpi_pnp_handler = {

Re: [PATCH v1 0/4] ACPI: scan: Handle generic system devices directly

[Mario Limonciello]

On 12/15/25 7:31 AM, Rafael J. Wysocki wrote:
> Hi All,
> 
> As per the changelog of patch [1/4], there is a long-standing problem with ACPI
> device enumeration that if the given device has a compatible ID which is one of
> the generic system resource device IDs (PNP0C01 and PNP0C02), it will be claimed
> by the PNP scan handler and it will not be represented as a platform device, so
> it cannot be handled by a platform driver.
> 
> Obviously, PNP0C01 and PNP0C02 need to be removed from acpi_pnp_device_ids[] to
> address this problem, but it cannot be done without making any other changes
> due to the way the legacy PNP system driver works (see the changelog of patch
> [1/4] for details) and the approach used in patch [1/4] is to reserve the
> "motherboard resources" directly in the ACPI core without creating platform
> devices for "system" devices (whose _HID is either PNP0C01 or PNP0C02).  The
> new code doing works along the lines of the PNP system driver to avoid
> regressions.
> 
> The remaining patches in the series are simplifications that can be done on
> top of the first one.
> 
> Patches [2-3/4] remove platform device creation code from Intel HID and VBTN
> drivers that don't need to worry about _CID including PNP0C02 any more.
> 
> Patch [4/4] drops acpi_nonpnp_device_ids[] that was added as a workaround for
> the issue at hand in the first place.
> 
> Thanks!
> 
> 
> 
> 

For the series:

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>