[PATCH] ACPI/Thermal: Add check of "_TZD" availability and evaluating result

[PATCH] ACPI/Thermal: Add check of "_TZD" availability and evaluating result

[tianyu.lan]

From: Lan Tianyu <tianyu.lan@intel.com>

Some machines don't provide _TZD. So add check of "_TZD" availability
before doing futher operations. This patch also adds the check of
evaluating result.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/acpi/thermal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 09f9340..6809fbd 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -492,11 +492,12 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 			break;
 	}
 
-	if (flag & ACPI_TRIPS_DEVICES) {
+	if ((flag & ACPI_TRIPS_DEVICES)
+			&& acpi_has_method(tz->device->handle, "_TZD")) {
 		memset(&devices, 0, sizeof(struct acpi_handle_list));
 		status = acpi_evaluate_reference(tz->device->handle, "_TZD",
 						NULL, &devices);
-		if (memcmp(&tz->devices, &devices,
+		if (ACPI_SUCCESS(status) && memcmp(&tz->devices, &devices,
 				sizeof(struct acpi_handle_list))) {
 			memcpy(&tz->devices, &devices,
 				sizeof(struct acpi_handle_list));
-- 
1.8.4.rc0.1.g8f6a3e5.dirty

Re: [PATCH] ACPI/Thermal: Add check of "_TZD" availability and evaluating result

[Rafael J. Wysocki]

On Saturday, August 24, 2013 05:01:28 PM tianyu.lan@intel.com wrote:
> From: Lan Tianyu <tianyu.lan@intel.com>
> 
> Some machines don't provide _TZD. So add check of "_TZD" availability
> before doing futher operations. This patch also adds the check of
> evaluating result.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  drivers/acpi/thermal.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 09f9340..6809fbd 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -492,11 +492,12 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
>  			break;
>  	}
>  
> -	if (flag & ACPI_TRIPS_DEVICES) {
> +	if ((flag & ACPI_TRIPS_DEVICES)
> +			&& acpi_has_method(tz->device->handle, "_TZD")) {

Please arrange the whitespace here like this:

	if ((flag & ACPI_TRIPS_DEVICES)
	    && acpi_has_method(tz->device->handle, "_TZD")) {


>  		memset(&devices, 0, sizeof(struct acpi_handle_list));
>  		status = acpi_evaluate_reference(tz->device->handle, "_TZD",
>  						NULL, &devices);
> -		if (memcmp(&tz->devices, &devices,
> +		if (ACPI_SUCCESS(status) && memcmp(&tz->devices, &devices,
>  				sizeof(struct acpi_handle_list))) {

I wonder why you can't use "tz->device == devices" here instead of the memcmp?

And also "tz->devices = devices" below for that matter.

>  			memcpy(&tz->devices, &devices,
>  				sizeof(struct acpi_handle_list));

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI/Thermal: Add check of "_TZD" availability and evaluating result

[Lan Tianyu]

Hi Rafael:
           Thanks for review.

2013/8/24 Rafael J. Wysocki <rjw@sisk.pl>:
> On Saturday, August 24, 2013 05:01:28 PM tianyu.lan@intel.com wrote:
>> From: Lan Tianyu <tianyu.lan@intel.com>
>>
>> Some machines don't provide _TZD. So add check of "_TZD" availability
>> before doing futher operations. This patch also adds the check of
>> evaluating result.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  drivers/acpi/thermal.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>> index 09f9340..6809fbd 100644
>> --- a/drivers/acpi/thermal.c
>> +++ b/drivers/acpi/thermal.c
>> @@ -492,11 +492,12 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
>>                       break;
>>       }
>>
>> -     if (flag & ACPI_TRIPS_DEVICES) {
>> +     if ((flag & ACPI_TRIPS_DEVICES)
>> +                     && acpi_has_method(tz->device->handle, "_TZD")) {
>
> Please arrange the whitespace here like this:
>
>         if ((flag & ACPI_TRIPS_DEVICES)
>             && acpi_has_method(tz->device->handle, "_TZD")) {
>

Ok. I will update.

>
>>               memset(&devices, 0, sizeof(struct acpi_handle_list));
>>               status = acpi_evaluate_reference(tz->device->handle, "_TZD",
>>                                               NULL, &devices);
>> -             if (memcmp(&tz->devices, &devices,
>> +             if (ACPI_SUCCESS(status) && memcmp(&tz->devices, &devices,
>>                               sizeof(struct acpi_handle_list))) {
>
> I wonder why you can't use "tz->device == devices" here instead of the memcmp?

After changing like this, compiling failed with the following warning.

drivers/acpi/thermal.c: In function 'acpi_thermal_trips_update':
drivers/acpi/thermal.c:500:43: error: invalid operands to binary ==
(have 'struct acpi_handle_list' and 'struct acpi_handle_list')
   if (ACPI_SUCCESS(status) && tz->devices == devices) {
                                                                    ^

>
> And also "tz->devices = devices" below for that matter.
>

This change is ok.


>>                       memcpy(&tz->devices, &devices,
>>                               sizeof(struct acpi_handle_list));
>
> Rafael
>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best regards
Tianyu Lan

Re: [PATCH] ACPI/Thermal: Add check of "_TZD" availability and evaluating result

[Rafael J. Wysocki]

On Sunday, August 25, 2013 02:39:30 PM Lan Tianyu wrote:
> Hi Rafael:
>            Thanks for review.
> 
> 2013/8/24 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Saturday, August 24, 2013 05:01:28 PM tianyu.lan@intel.com wrote:
> >> From: Lan Tianyu <tianyu.lan@intel.com>
> >>
> >> Some machines don't provide _TZD. So add check of "_TZD" availability
> >> before doing futher operations. This patch also adds the check of
> >> evaluating result.
> >>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >> ---
> >>  drivers/acpi/thermal.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> >> index 09f9340..6809fbd 100644
> >> --- a/drivers/acpi/thermal.c
> >> +++ b/drivers/acpi/thermal.c
> >> @@ -492,11 +492,12 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
> >>                       break;
> >>       }
> >>
> >> -     if (flag & ACPI_TRIPS_DEVICES) {
> >> +     if ((flag & ACPI_TRIPS_DEVICES)
> >> +                     && acpi_has_method(tz->device->handle, "_TZD")) {
> >
> > Please arrange the whitespace here like this:
> >
> >         if ((flag & ACPI_TRIPS_DEVICES)
> >             && acpi_has_method(tz->device->handle, "_TZD")) {
> >
> 
> Ok. I will update.
> 
> >
> >>               memset(&devices, 0, sizeof(struct acpi_handle_list));
> >>               status = acpi_evaluate_reference(tz->device->handle, "_TZD",
> >>                                               NULL, &devices);
> >> -             if (memcmp(&tz->devices, &devices,
> >> +             if (ACPI_SUCCESS(status) && memcmp(&tz->devices, &devices,
> >>                               sizeof(struct acpi_handle_list))) {
> >
> > I wonder why you can't use "tz->device == devices" here instead of the memcmp?
> 
> After changing like this, compiling failed with the following warning.
> 
> drivers/acpi/thermal.c: In function 'acpi_thermal_trips_update':
> drivers/acpi/thermal.c:500:43: error: invalid operands to binary ==
> (have 'struct acpi_handle_list' and 'struct acpi_handle_list')
>    if (ACPI_SUCCESS(status) && tz->devices == devices) {
>                                                                     ^

OK, so please use memcmp(&tz->devices, &devices, sizeof(devices)).

> 
> >
> > And also "tz->devices = devices" below for that matter.
> >
> 
> This change is ok.

Thanks,
Rafael