Hi Rui, On 8/21/2017 8:37 AM, Zhang Rui wrote: > On Fri, 2017-08-18 at 16:31 -0600, Prakash, Prashanth wrote: >> 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 > Yes I agree. > From userspace point of view, > with you patch, userspace can get the content of _STR by > cat /sys/class/thermal/thermal_zoneX/desc > > And what I mean is that, userspace can already get the same information > by > cat /sys/class/thermal/thermal_zoneX/device/description > even without the patch. > > >> - 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. > if the sensor is registered by native driver, I think .get_desc() is > useful. > But if you want to hack the dsdt to get it enumerated via ACPI, then my > approach still works without the patch. :) Yes, it works :) and agree we should drop these patch. Sorry, I should have checked more thoroughly before posting. Thanks for the 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
