Am 27.11.25 um 19:22 schrieb Rafael J. Wysocki:

On Sat, Nov 22, 2025 at 3:18 PM Armin Wolf <[email protected]> wrote:
Am 21.11.25 um 21:35 schrieb Rafael J. Wysocki:

On Thu, Nov 20, 2025 at 4:41 AM Armin Wolf <[email protected]> wrote:
[...]

---
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 link
That 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).

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.

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.

        ACPI: fan: Stop creating "device" sysfs link
        ACPI: video: Stop creating "device" sysfs link
Analogously 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.

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).

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.

        thermal: core: Set parent device in thermal_cooling_device_register()
        ACPI: thermal: Stop creating "device" sysfs link
And 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.

        thermal: core: Allow setting the parent device of thermal zone devices
I'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

Reply via email to