* [drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
@ 2008-02-04 22:07 Roel Kluin
2008-02-04 22:13 ` Roland Dreier
0 siblings, 1 reply; 12+ messages in thread
From: Roel Kluin @ 2008-02-04 22:07 UTC (permalink / raw)
To: len.brown, ibm-acpi; +Cc: ibm-acpi-devel, linux-acpi, lkml
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 */
?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-04 22:07 [drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) Roel Kluin @ 2008-02-04 22:13 ` Roland Dreier 2008-02-04 23:24 ` [PATCH][drivers/misc/thinkpad_acpi.c] " Roel Kluin 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2008-02-04 22:13 UTC (permalink / raw) To: Roel Kluin; +Cc: len.brown, ibm-acpi, ibm-acpi-devel, linux-acpi, lkml > /* 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). ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-04 22:13 ` Roland Dreier @ 2008-02-04 23:24 ` Roel Kluin 2008-02-05 5:05 ` Henrique de Moraes Holschuh 2008-02-07 6:17 ` Len Brown 0 siblings, 2 replies; 12+ messages in thread From: Roel Kluin @ 2008-02-04 23:24 UTC (permalink / raw) To: Roland Dreier; +Cc: len.brown, ibm-acpi, ibm-acpi-devel, linux-acpi, lkml 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 <12o3l@tiscali.nl> --- 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)) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-04 23:24 ` [PATCH][drivers/misc/thinkpad_acpi.c] " Roel Kluin @ 2008-02-05 5:05 ` Henrique de Moraes Holschuh 2008-02-05 8:34 ` Roel Kluin 2008-02-07 6:17 ` Len Brown 1 sibling, 1 reply; 12+ messages in thread From: Henrique de Moraes Holschuh @ 2008-02-05 5:05 UTC (permalink / raw) To: Roel Kluin; +Cc: Roland Dreier, len.brown, ibm-acpi-devel, linux-acpi, lkml 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 <12o3l@tiscali.nl> > --- > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-05 5:05 ` Henrique de Moraes Holschuh @ 2008-02-05 8:34 ` Roel Kluin 2008-02-06 23:07 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: Roel Kluin @ 2008-02-05 8:34 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Roland Dreier, len.brown, ibm-acpi-devel, linux-acpi, lkml, stable, Greg KH 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 <12o3l@tiscali.nl> >> --- >> 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 stable@kernel.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-05 8:34 ` Roel Kluin @ 2008-02-06 23:07 ` Greg KH 2008-02-06 23:48 ` Roel Kluin 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2008-02-06 23:07 UTC (permalink / raw) To: Roel Kluin Cc: Henrique de Moraes Holschuh, Roland Dreier, len.brown, ibm-acpi-devel, linux-acpi, lkml, stable 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 <12o3l@tiscali.nl> > >> --- > >> 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 stable@kernel.org 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-06 23:07 ` Greg KH @ 2008-02-06 23:48 ` Roel Kluin 2008-02-07 1:18 ` Henrique de Moraes Holschuh 2008-02-07 5:55 ` Greg KH 0 siblings, 2 replies; 12+ messages in thread From: Roel Kluin @ 2008-02-06 23:48 UTC (permalink / raw) To: Greg KH Cc: Henrique de Moraes Holschuh, Roland Dreier, len.brown, ibm-acpi-devel, linux-acpi, lkml, stable 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 stable@kernel.org > > 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 <12o3l@tiscali.nl> --- 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)) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-06 23:48 ` Roel Kluin @ 2008-02-07 1:18 ` Henrique de Moraes Holschuh 2008-02-07 7:28 ` [stable] " Greg KH 2008-02-07 5:55 ` Greg KH 1 sibling, 1 reply; 12+ messages in thread From: Henrique de Moraes Holschuh @ 2008-02-07 1:18 UTC (permalink / raw) To: Roel Kluin Cc: Greg KH, Roland Dreier, len.brown, ibm-acpi-devel, linux-acpi, lkml, stable 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 stable@k.o 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [stable] [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-07 1:18 ` Henrique de Moraes Holschuh @ 2008-02-07 7:28 ` Greg KH 0 siblings, 0 replies; 12+ messages in thread From: Greg KH @ 2008-02-07 7:28 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Roel Kluin, len.brown, linux-acpi, Roland Dreier, lkml, ibm-acpi-devel, stable 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 stable@k.o when it > hits mainline. Sounds good, I've dropped it from my queue, so please resend it when it hits Linus's tree to stable@kernel.org if you want it there. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [stable] [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-06 23:48 ` Roel Kluin 2008-02-07 1:18 ` Henrique de Moraes Holschuh @ 2008-02-07 5:55 ` Greg KH 1 sibling, 0 replies; 12+ messages in thread From: Greg KH @ 2008-02-07 5:55 UTC (permalink / raw) To: Roel Kluin Cc: len.brown, ibm-acpi-devel, Roland Dreier, lkml, linux-acpi, Henrique de Moraes Holschuh, stable 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 stable@kernel.org > > > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) 2008-02-04 23:24 ` [PATCH][drivers/misc/thinkpad_acpi.c] " Roel Kluin 2008-02-05 5:05 ` Henrique de Moraes Holschuh @ 2008-02-07 6:17 ` Len Brown 1 sibling, 0 replies; 12+ messages in thread From: Len Brown @ 2008-02-07 6:17 UTC (permalink / raw) To: Roel Kluin Cc: Roland Dreier, len.brown, ibm-acpi, ibm-acpi-devel, linux-acpi, lkml 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 <12o3l@tiscali.nl> > --- > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'
@ 2008-02-04 21:59 Roel kluin
0 siblings, 0 replies; 12+ messages in thread
From: Roel kluin @ 2008-02-04 21:59 UTC (permalink / raw)
To: ibm-acpi, len.brown; +Cc: linux-acpi, ibm-acpi-devel, lkml
in drivers/misc/thinkpad_acpi.c, lines 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 nonsense duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'.
should this be changed to:
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
else
level |= 4; /* safety min speed 4 */
or maybe
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
if (level & TP_EC_FAN_FULLSPEED)
level |= 4; /* safety min speed 4 */
?
^ permalink raw reply [flat|nested] 12+ messages in threadend of thread, other threads:[~2008-02-07 7:25 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-02-04 22:07 [drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED) Roel Kluin 2008-02-04 22:13 ` Roland Dreier 2008-02-04 23:24 ` [PATCH][drivers/misc/thinkpad_acpi.c] " Roel Kluin 2008-02-05 5:05 ` Henrique de Moraes Holschuh 2008-02-05 8:34 ` Roel Kluin 2008-02-06 23:07 ` Greg KH 2008-02-06 23:48 ` Roel Kluin 2008-02-07 1:18 ` Henrique de Moraes Holschuh 2008-02-07 7:28 ` [stable] " Greg KH 2008-02-07 5:55 ` Greg KH 2008-02-07 6:17 ` Len Brown -- strict thread matches above, loose matches on Subject: below -- 2008-02-04 21:59 [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test 'if (level & TP_EC_FAN_FULLSPEED)' Roel kluin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox