All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Guinot <simon@sequanux.org>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [lm-sensors] [PATCH v2] hwmon: add generic GPIO fan driver
Date: Thu, 21 Oct 2010 21:59:29 +0000	[thread overview]
Message-ID: <20101021215928.GS29120@kw.sim.vm.gnt> (raw)
In-Reply-To: <20101021144346.GA26460@ericsson.com>


[-- Attachment #1.1: Type: text/plain, Size: 1314 bytes --]

Hi Guenter,

On Thu, Oct 21, 2010 at 07:43:46AM -0700, Guenter Roeck wrote:
> > > 
> > > The combination of DIV_ROUND_UP() and DIV_ROUND_CLOSEST() causes inconsistency.
> > > 
> > > Assume num_speed = 8, pwm is set to 128.
> > > 
> > > set: 128 * (8 - 1) / 255 = 3.513 ==> 4
> > > get: 4 * 255 / (8 - 1) = 145.7 ==> 146
> > > set: 146 * (8 - 1) / 255 = 4.007 ==> 5
> > > get: 5 * 255 / (8 - 1) = 182.142 ==> 182
> > > set: 182 * (8 - 1) / 255 = 4.996 ==> 5
> > > 
> > > Unless there is a really good reason to use DIV_ROUND_UP(), you might
> > > want to use DIV_ROUND_CLOSEST() instead.
> > 
> > This choice is coherent with the rpm interface one and the reason is the
> > same: start the fan even with a low value. In your example, 36 is first
> > speed threshold.
> > 
> Yes, but here it causes an inconsistency between setting and reporting.
> I don't expect the speed to change if I set the same value that was read.
> Exactly this happens if one writes 146 in my example. That is much worse
> than a potential startup problem, or the observation that pwm values below X 
> don't start the fan.

Mmm. Convert a speed index into a low round pwm value (and not use
DIV_ROUND_CLOSEST() at all) fix the inconsistency too. If you agree,
I would prefer this option.

Thanks,

Simon

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: simon@sequanux.org (Simon Guinot)
To: linux-arm-kernel@lists.infradead.org
Subject: [lm-sensors] [PATCH v2] hwmon: add generic GPIO fan driver
Date: Thu, 21 Oct 2010 21:59:29 +0000	[thread overview]
Message-ID: <20101021215928.GS29120@kw.sim.vm.gnt> (raw)
In-Reply-To: <20101021144346.GA26460@ericsson.com>

Hi Guenter,

On Thu, Oct 21, 2010 at 07:43:46AM -0700, Guenter Roeck wrote:
> > > 
> > > The combination of DIV_ROUND_UP() and DIV_ROUND_CLOSEST() causes inconsistency.
> > > 
> > > Assume num_speed = 8, pwm is set to 128.
> > > 
> > > set: 128 * (8 - 1) / 255 = 3.513 ==> 4
> > > get: 4 * 255 / (8 - 1) = 145.7 ==> 146
> > > set: 146 * (8 - 1) / 255 = 4.007 ==> 5
> > > get: 5 * 255 / (8 - 1) = 182.142 ==> 182
> > > set: 182 * (8 - 1) / 255 = 4.996 ==> 5
> > > 
> > > Unless there is a really good reason to use DIV_ROUND_UP(), you might
> > > want to use DIV_ROUND_CLOSEST() instead.
> > 
> > This choice is coherent with the rpm interface one and the reason is the
> > same: start the fan even with a low value. In your example, 36 is first
> > speed threshold.
> > 
> Yes, but here it causes an inconsistency between setting and reporting.
> I don't expect the speed to change if I set the same value that was read.
> Exactly this happens if one writes 146 in my example. That is much worse
> than a potential startup problem, or the observation that pwm values below X 
> don't start the fan.

Mmm. Convert a speed index into a low round pwm value (and not use
DIV_ROUND_CLOSEST() at all) fix the inconsistency too. If you agree,
I would prefer this option.

Thanks,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101021/6d57e75b/attachment.sig>

  reply	other threads:[~2010-10-21 21:59 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-20 16:37 [lm-sensors] [PATCH v2] hwmon: add generic GPIO fan driver Simon Guinot
2010-10-20 16:37 ` Simon Guinot
2010-10-20 16:37 ` [lm-sensors] " Simon Guinot
2010-10-20 16:37   ` Simon Guinot
2010-10-20 16:59   ` [lm-sensors] " Guenter Roeck
2010-10-20 16:59     ` Guenter Roeck
2010-10-20 19:10     ` [lm-sensors] " Simon Guinot
2010-10-20 19:10       ` Simon Guinot
2010-10-20 20:42       ` [lm-sensors] " Guenter Roeck
2010-10-20 20:42         ` Guenter Roeck
2010-10-20 19:18     ` [lm-sensors] " Russell King - ARM Linux
2010-10-20 19:18       ` Russell King - ARM Linux
2010-10-20 20:06       ` [lm-sensors] " Guenter Roeck
2010-10-20 20:06         ` Guenter Roeck
2010-10-21  6:41   ` [lm-sensors] " Guenter Roeck
2010-10-21  6:41     ` Guenter Roeck
2010-10-21 14:06     ` [lm-sensors] " Simon Guinot
2010-10-21 14:06       ` Simon Guinot
2010-10-21 14:43       ` [lm-sensors] " Guenter Roeck
2010-10-21 14:43         ` Guenter Roeck
2010-10-21 21:59         ` Simon Guinot [this message]
2010-10-21 21:59           ` [lm-sensors] " Simon Guinot
2010-10-21 22:16           ` Guenter Roeck
2010-10-21 22:16             ` Guenter Roeck
2010-10-21 22:35             ` Simon Guinot
2010-10-21 22:35               ` Simon Guinot
2010-10-21 22:44               ` [lm-sensors] [PATCH v3] " Simon Guinot
2010-10-21 22:44                 ` Simon Guinot
2010-10-22  1:51                 ` [lm-sensors] " Guenter Roeck
2010-10-22  1:51                   ` Guenter Roeck
2010-10-22 12:25                   ` [lm-sensors] " Simon Guinot
2010-10-22 12:25                     ` Simon Guinot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101021215928.GS29120@kw.sim.vm.gnt \
    --to=simon@sequanux.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.