in drivers/misc/thinkpad_acpi.c: 4137-4142 it reads:
/* safety net should the EC not support AUTO
* or FULLSPEED mode bits and just ignore them */
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
else if (level & TP_EC_FAN_FULLSPEED)
level |= 4; /* safety min speed 4 */
Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
this be replaced by
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
else
level |= 4; /* safety min speed 4 */
or
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
if (level & TP_EC_FAN_FULLSPEED)
level |= 4; /* safety min speed 4 */
?
> /* safety net should the EC not support AUTO
> * or FULLSPEED mode bits and just ignore them */
> if (level & TP_EC_FAN_FULLSPEED)
> level |= 7; /* safety min speed 7 */
> else if (level & TP_EC_FAN_FULLSPEED)
> level |= 4; /* safety min speed 4 */
>
> Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
> this be replaced by
Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
(based on the comment).
Roland Dreier wrote:
> > /* safety net should the EC not support AUTO
> > * or FULLSPEED mode bits and just ignore them */
> > if (level & TP_EC_FAN_FULLSPEED)
> > level |= 7; /* safety min speed 7 */
> > else if (level & TP_EC_FAN_FULLSPEED)
> > level |= 4; /* safety min speed 4 */
> >
> > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
> > this be replaced by
>
> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
> (based on the comment).
>
Thanks Roland, for your info
based on the comments in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad,
I think this should be modified like below:
ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
The Linux ThinkPad community is not positive that all ThinkPads that do
HFSP EC fan control do implement full-speed and auto modes, some of the
earlier ones supporting HFSP might not.
If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
lower three bits that set the fan level. And as thinkpad-acpi was leaving
these set to zero, it would stop(!) the fan, which is Not A Good Thing.
So, as a safety net, we now make sure to also set the fan level part of the
HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
mode.
--
second TP_EC_FAN_FULLSPEED should be P_EC_FAN_AUTO
Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index cf56647..3c323fe 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
* or FULLSPEED mode bits and just ignore them */
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
- else if (level & TP_EC_FAN_FULLSPEED)
+ else if (level & TP_EC_FAN_AUTO)
level |= 4; /* safety min speed 4 */
if (!acpi_ec_write(fan_status_offset, level))
On Tue, 05 Feb 2008, Roel Kluin wrote:
> Roland Dreier wrote:
> > > /* safety net should the EC not support AUTO
> > > * or FULLSPEED mode bits and just ignore them */
> > > if (level & TP_EC_FAN_FULLSPEED)
> > > level |= 7; /* safety min speed 7 */
> > > else if (level & TP_EC_FAN_FULLSPEED)
> > > level |= 4; /* safety min speed 4 */
> > >
> > > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
> > > this be replaced by
> >
> > Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
> > (based on the comment).
>
> Thanks Roland, for your info
>
> based on the comments in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad,
> I think this should be modified like below:
>
> ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
> The Linux ThinkPad community is not positive that all ThinkPads that do
> HFSP EC fan control do implement full-speed and auto modes, some of the
> earlier ones supporting HFSP might not.
>
> If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
> lower three bits that set the fan level. And as thinkpad-acpi was leaving
> these set to zero, it would stop(!) the fan, which is Not A Good Thing.
> So, as a safety net, we now make sure to also set the fan level part of the
> HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
> mode.
> --
> second TP_EC_FAN_FULLSPEED should be P_EC_FAN_AUTO
>
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
> index cf56647..3c323fe 100644
> --- a/drivers/misc/thinkpad_acpi.c
> +++ b/drivers/misc/thinkpad_acpi.c
> @@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
> * or FULLSPEED mode bits and just ignore them */
> if (level & TP_EC_FAN_FULLSPEED)
> level |= 7; /* safety min speed 7 */
> - else if (level & TP_EC_FAN_FULLSPEED)
> + else if (level & TP_EC_FAN_AUTO)
> level |= 4; /* safety min speed 4 */
>
> if (!acpi_ec_write(fan_status_offset, level))
ACK. This needs to be sent to stable as well. I think both 2.6.22 and
2.6.23 need this patch.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
Henrique de Moraes Holschuh wrote:
> On Tue, 05 Feb 2008, Roel Kluin wrote:
>> Roland Dreier wrote:
>>> > /* safety net should the EC not support AUTO
>>> > * or FULLSPEED mode bits and just ignore them */
>>> > if (level & TP_EC_FAN_FULLSPEED)
>>> > level |= 7; /* safety min speed 7 */
>>> > else if (level & TP_EC_FAN_FULLSPEED)
>>> > level |= 4; /* safety min speed 4 */
>>> >
>>> > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
>>> > this be replaced by
>>>
>>> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
>>> (based on the comment).
>> Thanks Roland, for your info
>>
>> based on the comments in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad,
>> I think this should be modified like below:
>>
>> ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
>> The Linux ThinkPad community is not positive that all ThinkPads that do
>> HFSP EC fan control do implement full-speed and auto modes, some of the
>> earlier ones supporting HFSP might not.
>>
>> If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
>> lower three bits that set the fan level. And as thinkpad-acpi was leaving
>> these set to zero, it would stop(!) the fan, which is Not A Good Thing.
>> So, as a safety net, we now make sure to also set the fan level part of the
>> HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
>> mode.
>> --
>> second TP_EC_FAN_FULLSPEED should be P_EC_FAN_AUTO
>>
>>
>> Signed-off-by: Roel Kluin <[email protected]>
>> ---
>> diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
>> index cf56647..3c323fe 100644
>> --- a/drivers/misc/thinkpad_acpi.c
>> +++ b/drivers/misc/thinkpad_acpi.c
>> @@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
>> * or FULLSPEED mode bits and just ignore them */
>> if (level & TP_EC_FAN_FULLSPEED)
>> level |= 7; /* safety min speed 7 */
>> - else if (level & TP_EC_FAN_FULLSPEED)
>> + else if (level & TP_EC_FAN_AUTO)
>> level |= 4; /* safety min speed 4 */
>>
>> if (!acpi_ec_write(fan_status_offset, level))
>
> ACK. This needs to be sent to stable as well. I think both 2.6.22 and
> 2.6.23 need this patch.
>
CC [email protected]
On Tue, Feb 05, 2008 at 09:34:54AM +0100, Roel Kluin wrote:
> Henrique de Moraes Holschuh wrote:
> > On Tue, 05 Feb 2008, Roel Kluin wrote:
> >> Roland Dreier wrote:
> >>> > /* safety net should the EC not support AUTO
> >>> > * or FULLSPEED mode bits and just ignore them */
> >>> > if (level & TP_EC_FAN_FULLSPEED)
> >>> > level |= 7; /* safety min speed 7 */
> >>> > else if (level & TP_EC_FAN_FULLSPEED)
> >>> > level |= 4; /* safety min speed 4 */
> >>> >
> >>> > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
> >>> > this be replaced by
> >>>
> >>> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
> >>> (based on the comment).
> >> Thanks Roland, for your info
> >>
> >> based on the comments in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad,
> >> I think this should be modified like below:
> >>
> >> ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
> >> The Linux ThinkPad community is not positive that all ThinkPads that do
> >> HFSP EC fan control do implement full-speed and auto modes, some of the
> >> earlier ones supporting HFSP might not.
> >>
> >> If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
> >> lower three bits that set the fan level. And as thinkpad-acpi was leaving
> >> these set to zero, it would stop(!) the fan, which is Not A Good Thing.
> >> So, as a safety net, we now make sure to also set the fan level part of the
> >> HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
> >> mode.
> >> --
> >> second TP_EC_FAN_FULLSPEED should be P_EC_FAN_AUTO
> >>
> >>
> >> Signed-off-by: Roel Kluin <[email protected]>
> >> ---
> >> diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
> >> index cf56647..3c323fe 100644
> >> --- a/drivers/misc/thinkpad_acpi.c
> >> +++ b/drivers/misc/thinkpad_acpi.c
> >> @@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
> >> * or FULLSPEED mode bits and just ignore them */
> >> if (level & TP_EC_FAN_FULLSPEED)
> >> level |= 7; /* safety min speed 7 */
> >> - else if (level & TP_EC_FAN_FULLSPEED)
> >> + else if (level & TP_EC_FAN_AUTO)
> >> level |= 4; /* safety min speed 4 */
> >>
> >> if (!acpi_ec_write(fan_status_offset, level))
> >
> > ACK. This needs to be sent to stable as well. I think both 2.6.22 and
> > 2.6.23 need this patch.
> >
>
> CC [email protected]
Please send us a real copy of the patch, when it goes into Linus's tree,
so we know what to apply, and that it is safe to do so.
thanks,
greg k-h
Greg KH wrote:
> On Tue, Feb 05, 2008 at 09:34:54AM +0100, Roel Kluin wrote:
>> Henrique de Moraes Holschuh wrote:
>>> On Tue, 05 Feb 2008, Roel Kluin wrote:
>>>> Roland Dreier wrote:
>>>>> > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
>>>>> > this be replaced by
>>>>>
>>>>> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
>>>>> (based on the comment).
>>>> Thanks Roland, for your info
>>> ACK. This needs to be sent to stable as well. I think both 2.6.22 and
>>> 2.6.23 need this patch.
>>>
>> CC [email protected]
>
> Please send us a real copy of the patch, when it goes into Linus's tree,
> so we know what to apply, and that it is safe to do so.
It's a oneliner and the patch is from linus' tree.
---
in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad, a safety net for TPEC
fan control mode was added. Quoting that patch:
"The Linux ThinkPad community is not positive that all ThinkPads that do
HFSP EC fan control do implement full-speed and auto modes, some of the
earlier ones supporting HFSP might not.
If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
lower three bits that set the fan level. And as thinkpad-acpi was leaving
these set to zero, it would stop(!) the fan, which is Not A Good Thing.
So, as a safety net, we now make sure to also set the fan level part of the
HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
mode."
However, in the section below the test for the FULL-SPEED bits was not added:
the AUTO bits were tested twice. This patch corrects this and ensures that
the fan level part of the HFSP register is set to a minimum of speed 4 for
auto mode.
Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index cf56647..3c323fe 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
* or FULLSPEED mode bits and just ignore them */
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
- else if (level & TP_EC_FAN_FULLSPEED)
+ else if (level & TP_EC_FAN_AUTO)
level |= 4; /* safety min speed 4 */
if (!acpi_ec_write(fan_status_offset, level))
On Thu, 07 Feb 2008, Roel Kluin wrote:
> It's a oneliner and the patch is from linus' tree.
Roel, better to just let me and Len Brown handle it. I will add your patch
to my thinkpad-acpi queue and send it to Len soon, along with other simple
patches that are pending. Either I or Len will notify [email protected] when it
hits mainline.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Thu, Feb 07, 2008 at 12:48:24AM +0100, Roel Kluin wrote:
> Greg KH wrote:
> > On Tue, Feb 05, 2008 at 09:34:54AM +0100, Roel Kluin wrote:
> >> Henrique de Moraes Holschuh wrote:
> >>> On Tue, 05 Feb 2008, Roel Kluin wrote:
> >>>> Roland Dreier wrote:
>
> >>>>> > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
> >>>>> > this be replaced by
> >>>>>
> >>>>> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
> >>>>> (based on the comment).
> >>>> Thanks Roland, for your info
>
> >>> ACK. This needs to be sent to stable as well. I think both 2.6.22 and
> >>> 2.6.23 need this patch.
> >>>
> >> CC [email protected]
> >
> > Please send us a real copy of the patch, when it goes into Linus's tree,
> > so we know what to apply, and that it is safe to do so.
>
> It's a oneliner and the patch is from linus' tree.
> ---
> in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad, a safety net for TPEC
> fan control mode was added. Quoting that patch:
I don't see this patch in Linus's tree, I see the patch you are
patching, but not this one :)
Can you tell me the git id of the patch you wish to go into the stable
tree, and send the whole thing to us?
thanks,
greg k-h
Applied.
thanks,
-Len
On Monday 04 February 2008 18:24, Roel Kluin wrote:
> Roland Dreier wrote:
> > > /* safety net should the EC not support AUTO
> > > * or FULLSPEED mode bits and just ignore them */
> > > if (level & TP_EC_FAN_FULLSPEED)
> > > level |= 7; /* safety min speed 7 */
> > > else if (level & TP_EC_FAN_FULLSPEED)
> > > level |= 4; /* safety min speed 4 */
> > >
> > > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
> > > this be replaced by
> >
> > Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
> > (based on the comment).
> >
>
> Thanks Roland, for your info
>
> based on the comments in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad,
> I think this should be modified like below:
>
> ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
> The Linux ThinkPad community is not positive that all ThinkPads that do
> HFSP EC fan control do implement full-speed and auto modes, some of the
> earlier ones supporting HFSP might not.
>
> If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
> lower three bits that set the fan level. And as thinkpad-acpi was leaving
> these set to zero, it would stop(!) the fan, which is Not A Good Thing.
> So, as a safety net, we now make sure to also set the fan level part of the
> HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
> mode.
> --
> second TP_EC_FAN_FULLSPEED should be P_EC_FAN_AUTO
>
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
> index cf56647..3c323fe 100644
> --- a/drivers/misc/thinkpad_acpi.c
> +++ b/drivers/misc/thinkpad_acpi.c
> @@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
> * or FULLSPEED mode bits and just ignore them */
> if (level & TP_EC_FAN_FULLSPEED)
> level |= 7; /* safety min speed 7 */
> - else if (level & TP_EC_FAN_FULLSPEED)
> + else if (level & TP_EC_FAN_AUTO)
> level |= 4; /* safety min speed 4 */
>
> if (!acpi_ec_write(fan_status_offset, level))
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Wed, Feb 06, 2008 at 11:18:50PM -0200, Henrique de Moraes Holschuh wrote:
> On Thu, 07 Feb 2008, Roel Kluin wrote:
> > It's a oneliner and the patch is from linus' tree.
>
> Roel, better to just let me and Len Brown handle it. I will add your patch
> to my thinkpad-acpi queue and send it to Len soon, along with other simple
> patches that are pending. Either I or Len will notify [email protected] when it
> hits mainline.
Sounds good, I've dropped it from my queue, so please resend it when it
hits Linus's tree to [email protected] if you want it there.
thanks,
greg k-h