linux-kernel - Re: [PATCH 6/6] hwmon: pwm-fan: Update the duty cycle inorder to control the pwm-fan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 8 Apr 2015 21:32:05 +0530
From:	Anand Moon <linux.amoon@...il.com>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Lukasz Majewski <l.majewski@...sung.com>,
	Eduardo Valentin <edubezval@...il.com>,
	Sjoerd Simons <sjoerd.simons@...labora.co.uk>,
	Russell King <linux@....linux.org.uk>,
	Kukjin Kim <kgene@...nel.org>, devicetree@...r.kernel.org,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>, linux-pwm@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 6/6] hwmon: pwm-fan: Update the duty cycle inorder to
 control the pwm-fan

Hi Guenter,

Initially the board bootup the cooling level state is 0.
So update the duty cycle and this power off the fan.
As their is no state change the fan will not spin.

Once the temperature sensor is reached to alert temperature it changes state.
With the state change the fan cools the CPU and then stop's

I have observed this state change with tmon utility in linux/tools/thermal/tmon/

-Anand Moon

On 8 April 2015 at 21:02, Guenter Roeck <linux@...ck-us.net> wrote:
> On Wed, Apr 08, 2015 at 10:44:15AM +0200, Lukasz Majewski wrote:
>> Hi Anand,
>>
>> > Below changes depend on following patch.
>> > https://patchwork.kernel.org/patch/5944061/
>> >
>> > Update the pwm_config with duty then update the pwm_disable
>> > to poweroff the cpu fan.
>> >
>
> Unfortunately, the patch does not include an explanation why it is needed.
>
> The original code presumably did not update the duty cycle because
> pwm was about to be disabled anyway. That kind of made sense to me.
> Updating the duty cycle to 0 just to disable the pwm channel right
> afterwards does not immediately make sense.
>
> Given that, I would expect to see a rationale here. Why is this patch needed ?
> Does it fix a bug ? If yes, pelase describe the bug. If not, what is the
> purpose of this patch ?
>
> Maybe that is all explained in patch 0/6, which I was not copied on. Even
> if so, the reationale will be needed in the changelog to explain to future
> developers why this change was made.
>
> Thanks,
> Guenter
>
>> > Tested on OdroidXU3 board.
>> >
>> > Signed-off-by: Anand Moon <linux.amoon@...il.com>
>> > ---
>> >  drivers/hwmon/pwm-fan.c | 10 ++++------
>> >  1 file changed, 4 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
>> > index 7c83dc4..f25c841 100644
>> > --- a/drivers/hwmon/pwm-fan.c
>> > +++ b/drivers/hwmon/pwm-fan.c
>> > @@ -44,26 +44,24 @@ static int  __set_pwm(struct pwm_fan_ctx *ctx,
>> > unsigned long pwm) int ret = 0;
>> >
>> >     mutex_lock(&ctx->lock);
>> > +
>
> [ please refrain from unnecessary whitespace changes ]
>
>> >     if (ctx->pwm_value == pwm)
>> >             goto exit_set_pwm_err;
>> >
>> > -   if (pwm == 0) {
>> > -           pwm_disable(ctx->pwm);
>> > -           goto exit_set_pwm;
>> > -   }
>> > -
>> >     duty = DIV_ROUND_UP(pwm * (ctx->pwm->period - 1), MAX_PWM);
>> >     ret = pwm_config(ctx->pwm, duty, ctx->pwm->period);
>> >     if (ret)
>> >             goto exit_set_pwm_err;
>> >
>> > +   if (pwm == 0)
>> > +           pwm_disable(ctx->pwm);
>> > +
>> >     if (ctx->pwm_value == 0) {
>> >             ret = pwm_enable(ctx->pwm);
>> >             if (ret)
>> >                     goto exit_set_pwm_err;
>> >     }
>> >
>> > -exit_set_pwm:
>> >     ctx->pwm_value = pwm;
>> >  exit_set_pwm_err:
>> >     mutex_unlock(&ctx->lock);
>>
>> Reviewed-by: Lukasz Majewski <l.majewski@...sung.com>
>>
>> BTW: I've added Guenter to CC.
>>
>> --
>> Best regards,
>>
>> Lukasz Majewski
>>
>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/