Re: [PATCH RFC v3 01/21] ACPI: Only enumerate enabled (or functional) devices

From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	linux-pm@vger.kernel.org, loongarch@lists.linux.dev,
	linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, kvmarm@lists.linux.dev,
	x86@kernel.org, acpica-devel@lists.linuxfoundation.org,
	linux-csky@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-parisc@vger.kernel.org,
	Salil Mehta <salil.mehta@huawei.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	jianyong.wu@arm.com, justin.he@arm.com,
	James Morse <james.morse@arm.com>
Subject: Re: [PATCH RFC v3 01/21] ACPI: Only enumerate enabled (or functional) devices
Date: Fri, 15 Dec 2023 20:47:31 +0100	[thread overview]
Message-ID: <5760569.DvuYhMxLoT@kreacher> (raw)
In-Reply-To: <20231215161539.00000940@Huawei.com>

On Friday, December 15, 2023 5:15:39 PM CET Jonathan Cameron wrote:
> On Fri, 15 Dec 2023 15:31:55 +0000
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Thu, Dec 14, 2023 at 07:37:10PM +0100, Rafael J. Wysocki wrote:
> > > On Thu, Dec 14, 2023 at 7:16 PM Rafael J. Wysocki <rafael@kernel.org> wrote:  
> > > >
> > > > On Thu, Dec 14, 2023 at 7:10 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:  
> > > > > I guess we need something like:
> > > > >
> > > > >         if (device->status.present)
> > > > >                 return device->device_type != ACPI_BUS_TYPE_PROCESSOR ||
> > > > >                        device->status.enabled;
> > > > >         else
> > > > >                 return device->status.functional;
> > > > >
> > > > > so we only check device->status.enabled for processor-type devices?  
> > > >
> > > > Yes, something like this.  
> > > 
> > > However, that is not sufficient, because there are
> > > ACPI_BUS_TYPE_DEVICE devices representing processors.
> > > 
> > > I'm not sure about a clean way to do it ATM.  
> > 
> > Ok, how about:
> > 
> > static bool acpi_dev_is_processor(const struct acpi_device *device)
> > {
> > 	struct acpi_hardware_id *hwid;
> > 
> > 	if (device->device_type == ACPI_BUS_TYPE_PROCESSOR)
> > 		return true;
> > 
> > 	if (device->device_type != ACPI_BUS_TYPE_DEVICE)
> > 		return false;
> > 
> > 	list_for_each_entry(hwid, &device->pnp.ids, list)
> > 		if (!strcmp(ACPI_PROCESSOR_OBJECT_HID, hwid->id) ||
> > 		    !strcmp(ACPI_PROCESSOR_DEVICE_HID, hwid->id))
> > 			return true;
> > 
> > 	return false;
> > }
> > 
> > and then:
> > 
> > 	if (device->status.present)
> > 		return !acpi_dev_is_processor(device) || device->status.enabled;
> > 	else
> > 		return device->status.functional;
> > 
> > ?
> > 
> Changing it to CPU only for now makes sense to me and I think this code snippet should do the
> job.  Nice and simple.

Well, except that it does checks that are done elsewhere slightly
differently, which from the maintenance POV is not nice.

Maybe something like the appended patch (untested).

---
 drivers/acpi/acpi_processor.c |   11 +++++++++++
 drivers/acpi/internal.h       |    3 +++
 drivers/acpi/scan.c           |   24 +++++++++++++++++++++++-
 3 files changed, 37 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/acpi_processor.c
===================================================================

--- linux-pm.orig/drivers/acpi/acpi_processor.c
+++ linux-pm/drivers/acpi/acpi_processor.c
@@ -644,6 +644,17 @@ static struct acpi_scan_handler processo
 	},
 };
 
+bool acpi_device_is_processor(const struct acpi_device *adev)
+{
+	if (adev->device_type == ACPI_BUS_TYPE_PROCESSOR)
+		return true;
+
+	if (adev->device_type != ACPI_BUS_TYPE_DEVICE)
+		return false;
+
+	return acpi_scan_check_handler(adev, &processor_handler);
+}
+
 static int acpi_processor_container_attach(struct acpi_device *dev,
 					   const struct acpi_device_id *id)
 {
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -62,6 +62,8 @@ void acpi_sysfs_add_hotplug_profile(stru
 int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler,
 				       const char *hotplug_profile_name);
 void acpi_scan_hotplug_enabled(struct acpi_hotplug_profile *hotplug, bool val);
+bool acpi_scan_check_handler(const struct acpi_device *adev,
+			     struct acpi_scan_handler *handler);
 
 #ifdef CONFIG_DEBUG_FS
 extern struct dentry *acpi_debugfs_dir;
@@ -133,6 +135,7 @@ int acpi_bus_register_early_device(int t
 const struct acpi_device *acpi_companion_match(const struct device *dev);
 int __acpi_device_uevent_modalias(const struct acpi_device *adev,
 				  struct kobj_uevent_env *env);
+bool acpi_device_is_processor(const struct acpi_device *adev);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1938,6 +1938,19 @@ static bool acpi_scan_handler_matching(s
 	return false;
 }
 
+bool acpi_scan_check_handler(const struct acpi_device *adev,
+			     struct acpi_scan_handler *handler)
+{
+	struct acpi_hardware_id *hwid;
+
+	list_for_each_entry(hwid, &adev->pnp.ids, list) {
+		if (acpi_scan_handler_matching(handler, hwid->id, NULL))
+			return true;
+	}
+
+	return false;
+}
+
 static struct acpi_scan_handler *acpi_scan_match_handler(const char *idstr,
 					const struct acpi_device_id **matchid)
 {
@@ -2410,7 +2423,16 @@ bool acpi_dev_ready_for_enumeration(cons
 	if (device->flags.honor_deps && device->dep_unmet)
 		return false;
 
-	return acpi_device_is_present(device);
+	if (device->status.functional)
+		return true;
+
+	if (!device->status.present)
+		return false;
+
+	if (device->status.enabled)
+		return true; /* Fast path. */
+
+	return !acpi_device_is_processor(device);
 }
 EXPORT_SYMBOL_GPL(acpi_dev_ready_for_enumeration);
 




_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
