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. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
