On Thu, Nov 27, 2025 at 9:29 PM Armin Wolf <[email protected]> wrote: > > 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.
I think you mean device_add_class_symlinks(), but that's just for class devices. Of course, thermal devices are class devices, so they'll get those links if they get parents. Fair enough. > > 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). OK > >> 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. Let's set suspend aside for now, I think I've explained my viewpoint on this enough elsewhere. > > 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. No, I'm not. > 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. Maybe. > >>>> 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". Yes, there is. > This patch series exclusively deals with telling the driver core that "this > struct thermal_zone_device > depends on device X to be operational". Maybe let's take care of cooling devices first and get back to this later?
