Re: [RFC PATCH 6/6] Thermal: step_wise: set next cooling target explicitly

[Zhang Rui <rui.zhang@intel.com>]

On Tue, 2013-07-02 at 17:03 -0400, Eduardo Valentin wrote:
> On 17-06-2013 09:24, Zhang Rui wrote:
> > Set the next target state explicitly for each thermal trend,
> > in step_wise governor, to provide more readability, and to
> > follow the cooling algorithm description strictly.
> 
> 
> How about merging this one with patch 05?
> 
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/thermal/step_wise.c |    9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> > index 53b56e6..ffa553f 100644
> > --- a/drivers/thermal/step_wise.c
> > +++ b/drivers/thermal/step_wise.c
> > @@ -60,7 +60,6 @@ static unsigned long get_target_state(struct thermal_instance *instance,
> >  	 * cdev in use to determine the next_target.
> >  	 */
> >  	cdev->ops->get_cur_state(cdev, &cur_state);
> > -	next_target = instance->target;
> >  
> >  	switch (trend) {
> >  	case THERMAL_TREND_RAISING:
> > @@ -69,11 +68,14 @@ static unsigned long get_target_state(struct thermal_instance *instance,
> >  				    (cur_state + 1) : instance->upper;
> >  			if (next_target < instance->lower)
> >  				next_target = instance->lower;
> > -		}
> > +		} else
> > +			next_target = instance->target;
> >  		break;
> >  	case THERMAL_TREND_RAISE_FULL:
> >  		if (throttle)
> >  			next_target = instance->upper;
> > +		else
> > +			next_target = instance->target;
> >  		break;
> >  	case THERMAL_TREND_DROPPING:
> >  		if (throttle) {
> > @@ -99,7 +101,10 @@ static unsigned long get_target_state(struct thermal_instance *instance,
> >  		break;
> >  	case THERMAL_TREND_STABLE:
> >  		/* Do nothing */
> > +		next_target = instance->target;
> > +		break;
> >  	default:
> > +		next_target = instance->target;
> >  		break;
> 
> 
> The above could be rewritten like this:
>   	case THERMAL_TREND_STABLE:
>   		/* Do nothing */
>   	default:
>  		next_target = instance->target;
>   		break;
> 

sure I'm okay with your suggestion.
I wrote the code in this way because I want to keep the code follows the
description explicitly.

thanks,
rui
> >  	}
> >  
> > 
> 
>