Hi Rafael/Rui, On 8/17/2017 8:14 PM, Zhang Rui wrote: > On Fri, 2017-08-18 at 02:09 +0200, Rafael J. Wysocki wrote: >> On Wed, Aug 9, 2017 at 4:27 PM, Zhang Rui <rui.zhang@xxxxxxxxx> >> wrote: >>> Hi, Prakash, >>> >>> On Tue, 2017-08-08 at 10:01 -0600, Prakash, Prashanth wrote: >>>> Hi Rui, >>>> >>>> On 8/8/2017 2:23 AM, Zhang Rui wrote: >>>>> >>>>> On Fri, 2017-07-14 at 11:48 -0600, Prashanth Prakash wrote: >>>>>> >>>>>> Per ACPI 6.2 spec, platforms can optionally add a >>>>>> string(_STR) >>>>>> object within each thermal zone package which provides a user >>>>>> friendly name/description. >>>>>> >>>>>> Add support to parse the string object, which will be exposed >>>>>> to userspace by thermal framework. >>>>>> >>>>> is there any real request for this? >>>> Yes, Qualcomm server platforms adds these description strings. >>>>> >>>>> _STR is a generic control method for all the ACPI devices. >>>>> Thus I'm wondering, if really needed, should we expose this in >>>>> acpi >>>>> bus >>>>> instead? >>>> AFAIK, adding a _STR to any package was not explicitly allowed by >>>> the >>>> spec. >>>> Updates in APCI 6.2 made it legal to add an _STR object to >>>> thermal >>>> zone >>>> specifically, so added this support only to thermal zone. >>>> >>> I see that _STR is stated explicitly in 11.4.14, ACPI spec 6.2, but >>> according to section 6.1.10, "The _STR object evaluates to a >>> Unicode >>> string that describes the device or thermal zone. " >>> _STR is still a generic control method that can exist in any other >>> device scope. >>> >>> so to me, this is a optional but generic feature for all the ACPI >>> devices, and we don't have a solid reason that it should be part of >>> thermal sysfs I/F, thus a better solution to me is to expose this >>> as an >>> attribute of ACPI device, and we can link to the ACPI device from >>> thermal sysfs I/F in userspace. >>> >>> what do you think, Rafael? >> Since you have a ->get_desc method, I don't have a big problem with >> the approach here. >> >> I'm not particularly liking the "<not supported>" thing returned if >> _STR is not present, though. I will change the implementation such that if _STR object was not found then thermal_get_desc would return -ENXIO (or should it be different errno?). > No, actually I mean adding a new sysfs attribute under ACPI device > node, just like path/hid/status/adr, etc. Sorry Rui, I didn't read your earlier comment correctly. Thermal zone's _STR is useful in couple of scenarios(even if ACPI device containing the thermal_zone had a _STR object): - When we have more than 1 thermal sensors/zones under a device then this will allow us to differentiate them - When we have some thermal sensors that doesn't have ACPI device associated with it. For example: a shared L3 cache, an abstract region on SoC. In these cases we can add a thermal zone object in an appropriate place in dsdt and the associated _STR will allow us to provide a user friendly name/description. > Of course the attribute should be optional, depends on if the _STR > control methods exist or not. > > thanks, > rui >> Thanks, >> Rafael > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Thanks for your feedback! -- Regards, Prashanth -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html