On Tue, Jan 9, 2024 at 4:49 PM Russell King (Oracle) <linux@xxxxxxxxxxxxxxx> wrote: > > On Mon, Dec 18, 2023 at 09:17:34PM +0100, Rafael J. Wysocki wrote: > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@xxxxxxxxxxxxxxx> wrote: > > > > > > From: James Morse <james.morse@xxxxxxx> > > > > > > ACPI has two ways of describing processors in the DSDT. From ACPI v6.5, > > > 5.2.12: > > > > > > "Starting with ACPI Specification 6.3, the use of the Processor() object > > > was deprecated. Only legacy systems should continue with this usage. On > > > the Itanium architecture only, a _UID is provided for the Processor() > > > that is a string object. This usage of _UID is also deprecated since it > > > can preclude an OSPM from being able to match a processor to a > > > non-enumerable device, such as those defined in the MADT. From ACPI > > > Specification 6.3 onward, all processor objects for all architectures > > > except Itanium must now use Device() objects with an _HID of ACPI0007, > > > and use only integer _UID values." > > > > > > Also see https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#declaring-processors > > > > > > Duplicate descriptions are not allowed, the ACPI processor driver already > > > parses the UID from both devices and containers. acpi_processor_get_info() > > > returns an error if the UID exists twice in the DSDT. > > > > I'm not really sure how the above is related to the actual patch. > > > > > The missing probe for CPUs described as packages > > > > It is unclear what exactly is meant by "CPUs described as packages". > > > > From the patch, it looks like those would be Processor() objects > > defined under a processor container device. > > > > > creates a problem for > > > moving the cpu_register() calls into the acpi_processor driver, as CPUs > > > described like this don't get registered, leading to errors from other > > > subsystems when they try to add new sysfs entries to the CPU node. > > > (e.g. topology_sysfs_init()'s use of topology_add_dev() via cpuhp) > > > > > > To fix this, parse the processor container and call acpi_processor_add() > > > for each processor that is discovered like this. > > > > Discovered like what? > > > > > The processor container > > > handler is added with acpi_scan_add_handler(), so no detach call will > > > arrive. > > > > The above requires clarification too. > > The above comments... yea. As I didn't write the commit description, but > James did, and James has basically vanished, I don't think these can be > answered, short of rewriting the entire commit message, with me spending > a lot of time with the ACPI specification trying to get the terminology > right - because at lot of the above on the face of it seems to be things > to do with wrong terminology being used. > > I wasn't expecting this level of issues with this patch set, and I now > feel completely out of my depth with this series. I'm wondering whether > I should even continue with it, since I don't have the ACPI knowledge > to address a lot of these comments. Well, sorry about this. I met James at the LPC last year, so he seems to be still around, in some way at least..