Re: [PATCH RFC RESEND 0/8] thermal: core: Allow setting the parent device of thermal zone/cooling devices
From: Armin Wolf <W_Armin@gmx.de>
Date: 2025-11-27 20:29:28
Also in:
ath11k, dri-devel, imx, linux-acpi, linux-arm-kernel, linux-doc, linux-pci, linux-pm, linux-renesas-soc, linux-tegra, linux-wireless, lkml, netdev, platform-driver-x86
Am 27.11.25 um 19:22 schrieb Rafael J. Wysocki:
On Sat, Nov 22, 2025 at 3:18 PM Armin Wolf [off-list ref] wrote:quoted
Am 21.11.25 um 21:35 schrieb Rafael J. Wysocki:quoted
On Thu, Nov 20, 2025 at 4:41 AM Armin Wolf [off-list ref] wrote:[...]quoted
quoted
quoted
--- Armin Wolf (8): thermal: core: Allow setting the parent device of cooling devices thermal: core: Set parent device in thermal_of_cooling_device_register() ACPI: processor: Stop creating "device" sysfs linkThat link is not to the cooling devices' parent, but to the ACPI device object (a struct acpi_device) that corresponds to the parent. The parent of the cooling device should be the processor device, not its ACPI companion, so I'm not sure why there would be a conflict.From the perspective of the Linux device core, a parent device does not have to be a "physical" device. In the case of the ACPI processor driver, the ACPI device is used, so the cooling device registered by said driver belongs to the ACPI device.Well, that's a problem. A struct acpi_device should not be a parent of anything other than a struct acpi_device.
Understandable, in this case we should indeed use the the CPU device, especially since the fwnode associated with it already points to the correct ACPI processor object (at least on my machine).
quoted
I agree that using the Linux processor device would make more sense, but this will require changes inside the ACPI processor driver.So be it.
OK.
quoted
As for the "device" symlink: The conflict would be a naming conflict, as both "device" symlinks (the one created by the ACPI processor driver and the one created by the device core) will be created in the same directory (which is the directory of the cooling device).I see. But why is the new symlink needed in the first place? If the device has a parent, it will appear under that parent in /sys/devices/, won't it? Currently, all of the thermal class devices appear under /sys/devices/virtual/thermal/ because they have no parents and they all get a class parent kobject under /sys/devices/virtual/, as that's what get_device_parent() does. If they have real parents, they will appear under those parents, so why will the parents need to be pointed to additionally?
The "device" smylink is a comfort feature provided by the device core itself to allow user space application to traverse the device tree from bottom to top, like a double-linked list. We cannot disable the creation of this symlink, nor should we.
BTW, this means that the layout of /sys/devices/ will change when thermal devices get real parents. I'm not sure if this is a problem, but certainly something to note.
I know, most applications likely use /sys/class/thermal/, so they are not impacted by this. I will note this in the cover letter of the next revision.
quoted
quoted
quoted
ACPI: fan: Stop creating "device" sysfs link ACPI: video: Stop creating "device" sysfs linkAnalogously in the above two cases AFAICS. The parent of a cooling device should be a "physical" device object, like a platform device or a PCI device or similar, not a struct acpi_device (which in fact is not a device even).From the perspective of the Linux device core, a ACPI device is a perfectly valid device.The driver core is irrelevant here. As I said before, a struct acpi_device object should not be a parent of anything other than a struct acpi_device object. Those things are not devices and they cannot be used for representing PM dependencies, for example.quoted
I agree that using a platform device or PCI device is better, but this already happens inside the ACPI fan driver (platform device).So it should not happen there.
I meant that the ACPI fan driver already uses the platform device as the parent device of the cooling device, so the ACPI device is only used for interacting with the ACPI control methods (and registering sysfs attributes i think).
quoted
Only the ACPI video driver created a "device" sysfs link that points to the ACPI device instead of the PCI device. I just noticed that i accidentally changed this by using the PCI device as the parent device for the cooling device. If you want then we can keep this change.The PCI device should be its parent.
Alright, i will note this in the patch description.
quoted
quoted
quoted
thermal: core: Set parent device in thermal_cooling_device_register() ACPI: thermal: Stop creating "device" sysfs linkAnd this link is to the struct acpi_device representing the thermal zone itself.Correct, the ACPI thermal zone driver is a ACPI driver, meaning that he binds to ACPI devices. Because of this all (thermal zone) devices created by an instance of said driver are descendants of the ACPI device said instance is bound to. We can of course convert the ACPI thermal zone driver into a platform driver, but this would be a separate patch series.If you want parents, this needs to be done first, but I'm still not sure what the parent of a thermal zone would represent. In the ACPI case it is kind of easy - it would be the (platform) device corresponding to a given ThermalZone object in the ACPI namespace - but it only has a practical meaning if that device has a specific parent. For example, if the corresponding ThermalZone object is present in the \_SB scope, the presence of the thermal zone parent won't provide any additional information.
To the device core it will, as the platform device will need to be suspended after the thermal zone device has been suspended, among other things.
Unfortunately, the language in the specification isn't particularly helpful here: "Thermal zone objects should appear in the namespace under the portion of the system that comprises the thermal zone. For example, a thermal zone that is isolated to a docking station should be defined within the scope of the docking station device." To me "the portion of the system" is not too meaningful unless it is just one device without children. That's why _TZD has been added AFAICS.
I think you are confusing the parent device of the ThermalZone ACPI device with the parent device of the struct thermal_zone_device. I begin to wonder if mentioning the ACPI ThermalZone device together with the struct thermal_zone_device was a bad idea on my side xd.
quoted
quoted
quoted
thermal: core: Allow setting the parent device of thermal zone devicesI'm not sure if this is a good idea, at least until it is clear what the role of a thermal zone parent device should be.Take a look at my explanation with the Intel Wifi driver.I did and I think that you want the parent to be a device somehow associated with the thermal zone, but how exactly? What should that be in the Wifi driver case, the PCI device or something else? And what if the thermal zone affects multiple devices? Which of them (if any) would be its parent? And would it be consistent with the ACPI case described above? All of that needs consideration IMV.
I agree, but there is a difference between "this struct thermal_zone_device depends on device X to be operational" and "this thermal zone affects device X, device Y and device Z". This patch series exclusively deals with telling the driver core that "this struct thermal_zone_device depends on device X to be operational". Thanks, Armin Wolf