Re: Thermal:about a BUG

Re: Thermal:about a BUG

[Zhang Rui]

Hi,

thanks for reporting the problem.

On 三, 2013-11-20 at 17:07 +0800, xulinuxkernel@gmail.com wrote:
>  
> Hi  all 
> Recently I porting the thermal driver,I think there will be a bug
> following,I am Just not sure about it,
> In functon get_target_state (),
> ...
> case THERMAL_TREND_DROPPING:
> if (cur_state == instance->lower) {
> if (!throttle)
> next_target = THERMAL_NO_TARGET;
> } else {
>     next_target = cur_state - 1;
>     if (next_target > instance->upper)
>         next_target = instance->upper;
> }
> break;
> case THERMAL_TREND_DROP_FULL:
> if (cur_state == instance->lower) {
> if (!throttle)
>         next_target = THERMAL_NO_TARGET;
> } else
>     next_target = instance->lower;
> break;
> ..
>  
> For case THERMAL_TREND_DROPPING
> if cur_state=0,instance->lower=2,
> so next_state =0xffffffff ,so next_target > instance->upper will be
> right,
>  
> I think In this case will be wrong.
> and I think this will be okay.
>  
> if (cur_state <= instance->lower) {
> if (!throttle)
> next_target = THERMAL_NO_TARGET;
> } else {
>     next_target = cur_state - 1;
>     if (next_target > instance->upper)
>         next_target = instance->upper;
> }

this should fix the problem for you, plus we may also needs this change
for THERMAL_TREND_DROPPING_FULL case.
But my question would be:
How could a cooling device be in a cooling state that is lower than the
lower limit of an active thermal_instance?
This seems like that the cooling device' cooling state is changed beyond
thermal core, and I do not think the current code handles this situation
well. IMO, we need a bigger fix for this issue. Let me think about it
and reply to you later.

thanks,
rui

Re: Thermal:about a BUG

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

On 20-11-2013 23:33, Zhang Rui wrote:
> Hi,
> 
> thanks for reporting the problem.
> 
> On 三, 2013-11-20 at 17:07 +0800, xulinuxkernel@gmail.com wrote:
>>  
>> Hi  all 
>> Recently I porting the thermal driver,I think there will be a bug
>> following,I am Just not sure about it,
>> In functon get_target_state (),
>> ...
>> case THERMAL_TREND_DROPPING:
>> if (cur_state == instance->lower) {
>> if (!throttle)
>> next_target = THERMAL_NO_TARGET;
>> } else {
>>     next_target = cur_state - 1;
>>     if (next_target > instance->upper)
>>         next_target = instance->upper;
>> }
>> break;
>> case THERMAL_TREND_DROP_FULL:
>> if (cur_state == instance->lower) {
>> if (!throttle)
>>         next_target = THERMAL_NO_TARGET;
>> } else
>>     next_target = instance->lower;
>> break;
>> ..
>>  
>> For case THERMAL_TREND_DROPPING
>> if cur_state=0,instance->lower=2,
>> so next_state =0xffffffff ,so next_target > instance->upper will be
>> right,
>>  
>> I think In this case will be wrong.
>> and I think this will be okay.
>>  
>> if (cur_state <= instance->lower) {
>> if (!throttle)
>> next_target = THERMAL_NO_TARGET;
>> } else {
>>     next_target = cur_state - 1;
>>     if (next_target > instance->upper)
>>         next_target = instance->upper;
>> }
> 
> this should fix the problem for you, plus we may also needs this change
> for THERMAL_TREND_DROPPING_FULL case.
> But my question would be:
> How could a cooling device be in a cooling state that is lower than the
> lower limit of an active thermal_instance?
> This seems like that the cooling device' cooling state is changed beyond
> thermal core, and I do not think the current code handles this situation
> well. IMO, we need a bigger fix for this issue. Let me think about it
> and reply to you later.

I think it is better to investigate further before we come up with a fix.

Xu, can you please describe the thermal zone you are using? Also what
cooling device are you using? Is the cooling device in mainline?

Do you really see this error happening? Can you please describe the
scenario that leads to the error ?

> 
> thanks,
> rui
> 
> 
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]