On Fri, 2017-08-18 at 14:34 +0200, Rafael J. Wysocki wrote:

On Friday, August 18, 2017 4:14:47 AM CEST Zhang Rui wrote:

quoted

On Fri, 2017-08-18 at 02:09 +0200, Rafael J. Wysocki wrote:

quoted

On Wed, Aug 9, 2017 at 4:27 PM, Zhang Rui [off-list ref]
wrote:

quoted

Hi, Prakash,

On Tue, 2017-08-08 at 10:01 -0600, Prakash, Prashanth wrote:

quoted

Hi Rui,

On 8/8/2017 2:23 AM, Zhang Rui wrote:

quoted

On Fri, 2017-07-14 at 11:48 -0600, Prashanth Prakash wrote:

quoted

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.

quoted

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

No, actually I mean adding a new sysfs attribute under ACPI device
node, just like path/hid/status/adr, etc.

Yes, I understood that, but since the power supply framework has a
description callback, why not to use it really?

I just checked the code, and ACPI devices indeed have 'description'
sysfs attribute if there is _STR. Cool, that's what I'm proposing.

But your statement, "the power supply framework has a description
callback" still confuses me.

quoted

Of course the attribute should be optional, depends on if the _STR
control methods exist or not.

So what's the exact benefit from doing this over the approach the
$subject
patch?

hmmm, the subject patch introduces kernel code to get _STR content from
ACPI and then export it to userspace via a new thermal sysfs attribute.

And what I'm proposing is that, if the content of _STR is available
under ACPI device node, then we can easily get the information from
thermal sysfs I/F using symbol links, thus we don't need kernel changes
or new thermal sysfs attribute.

thanks,
rui

Thanks,
Rafael