All of lore.kernel.org
 help / color / mirror / Atom feed
From: Goffredo Baroncelli <kreijack@inwind.it>
To: Jean Delvare <jdelvare@suse.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linux-kernel@vger.kernel.org, bryan@whatroute.net
Subject: Re: [PATCH 3/4] Add the "verbose" module option.
Date: Mon, 04 Aug 2014 19:10:37 +0200	[thread overview]
Message-ID: <53DFBE8D.5050707@inwind.it> (raw)
In-Reply-To: <1407141986.4302.10.camel@chaos.site>

On 08/04/2014 10:46 AM, Jean Delvare wrote:
> Le Sunday 03 August 2014 à 18:36 +0200, Goffredo Baroncelli a écrit :
>> On 08/03/2014 05:52 PM, Jean Delvare wrote:
>>> On Sun, 03 Aug 2014 17:12:57 +0200, Goffredo Baroncelli wrote:
>>>> On 08/03/2014 04:12 PM, Jean Delvare wrote:
>>>>>> +	    (verbose > 0 && level >= 0)) {
>>>>>> +		print_temp("CPU-temp: ", temp );
>>>>>> +		if (casetemp)
>>>>>> +			print_temp(", Case: ", casetemp );
>>>>>> +		if (level >= 0)
>>>>>> +			printk(", Fan: %d (tuned %+d)\n", 11-level,
>>>>>> +				x.fan_level-level );
>>>>>> +		else
>>>>>> +			printk(", Fan: %d (tuned +0)\n",x.fan_level);
>>>>>
>>>>> I think you can do without the "tuned +0" which doesn't add much value.
>>>>
>>>> Me too. But the old driver does the same, so I preferred to 
>>>> leave it as is.
>>>
>>> I looked at the code again and no, I can't see the old code doing that.
>>> It has "tuned %+d" only in tune_fan() which is only called if
>>> level >= 0. The other printk (when tune_fan isn't called) doesn't have
>>> a "tuned" part.
>>
>> This is taken from an old log of a v3.2 kernel (no changes here):
>>
>> [  886.510879] CPU-temp: 55.4 C, Case: 33.1 C,  Fan: 0 (tuned -11)
>> [  910.522869] CPU-temp: 56.0 C, Case: 33.5 C,  Fan: 0 (tuned +0)
>> [  958.546880] CPU-temp: 57.0 C, Case: 34.1 C,  Fan: 3 (tuned +3)
>>
>> in the code if level <0, then there is no update in the log. But if
>> level >0 and level is equal to the previous one, this leads to
>> have "tuned +0"...
> 
> I agree with that.
> 
>> But I have to be honest: I have not fully understand how 
>> "level" is computed.
> 
> I agree with that too :/
> 
>> The printk without "(tuned %+d)" is never called because 
>> LOG_TEMP was #define(d) equal to 0.
> 
> And this is what your second printk is replacing. So it should not have
> the "(tuned *)" either.
> 
I removed the printk(s) from tune_fan(); the ones leaved replaced 
both the ones inside tune_fan() and the ones outside.

Anyway, Benjamin which is your opinion ? 
For me is equal to remove or to leave "(tune +0)" (when the tuning is equal to 0).
Jean think it is better to remove "(tune +0)" (when the tuning is equal to 0).
So if you haven't any objection I will remove it.

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

  reply	other threads:[~2014-08-04 17:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-01 14:00 [PATCH][v2] therm_windtunnel doesn't work properly on PowerMac G4 Goffredo Baroncelli
2014-08-01 14:00 ` [PATCH 1/4] Update drivers names to the ones invoked by i2c-powermac Goffredo Baroncelli
2014-08-01 14:00 ` [PATCH 2/4] Remove attach_method because un-used Goffredo Baroncelli
2014-08-01 14:00 ` [PATCH 3/4] Add the "verbose" module option Goffredo Baroncelli
2014-08-03 14:12   ` Jean Delvare
2014-08-03 15:12     ` Goffredo Baroncelli
2014-08-03 15:52       ` Jean Delvare
2014-08-03 16:36         ` Goffredo Baroncelli
2014-08-04  8:46           ` Jean Delvare
2014-08-04 17:10             ` Goffredo Baroncelli [this message]
2014-08-05  8:59               ` Jean Delvare
2014-08-01 14:00 ` [PATCH 4/4] Return the fan speed via sysfs Goffredo Baroncelli
2014-08-03 14:17   ` Jean Delvare
2014-08-03 15:27     ` Goffredo Baroncelli
2014-08-03 15:59       ` Jean Delvare
2014-08-03 16:42         ` Goffredo Baroncelli
2014-08-04  8:44           ` Jean Delvare

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=53DFBE8D.5050707@inwind.it \
    --to=kreijack@inwind.it \
    --cc=benh@kernel.crashing.org \
    --cc=bryan@whatroute.net \
    --cc=jdelvare@suse.de \
    --cc=linux-kernel@vger.kernel.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.