Re: [stable] [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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/ > -- 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/
Re: [stable] [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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)) -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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)) -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [stable] [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [stable] [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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 -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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] -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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] -- 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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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 -- 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/
[PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)
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/
[PATCH][drivers/misc/thinkpad_acpi.c] duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'
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 */ ? -- 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/
[PATCH][drivers/misc/thinkpad_acpi.c] duplicate test 'if (level TP_EC_FAN_FULLSPEED)'
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 */ ? -- 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/
[PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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/
Re: [PATCH][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)
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 -- 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/
