diff mbox

machintosh: select defaults for cooling

Message ID 20170714114629.1512-1-linus.walleij@linaro.org
State New
Headers show

Commit Message

Linus Walleij July 14, 2017, 11:46 a.m. UTC 
I have this pretty nasty problem when trying to boot up a fresh
openSuSE DVD on a PowerMac G5: the kernel by default does not have
CONFIG_WINDFARM_PM72 enabled, with the effect that the cooling
is not functioning.

The BIOS on the PowerMac G5 reacts to this by, after a grace
period when the BIOS has waited for the OS to take over, increasing
the fan speeds so it sounds like an airplane is in the room, and
after another grace period simply cutting the power to the
machine. This is done not beacuse the cooling is not working, but
because the BIOS is not recieving handover of cooling from the
OS, so it panics and give up. The problem has been reported by
Linux users online.

Needless to say, this makes it impossible to install the OS
before the machine turns itself off.

The g5_defconfig looks like this:
CONFIG_PMAC_SMU=y
CONFIG_WINDFARM=y
CONFIG_WINDFARM_PM81=y
CONFIG_WINDFARM_PM91=y
CONFIG_WINDFARM_PM112=y
CONFIG_WINDFARM_PM121=y

Notably PM72 is missing, making the PowerMac G5 fail.

The defconfig is not the right place to do this: it should be
done by default when selecting Mac support for PPC/PPC64 and
especially for the Macs CPUfreq driver. We select SMU by default
for PPC_PMAC64, WINDFARM by default on PPC_PMAC and all the
WINDFARM thermal managers by default if CPU_FREQ_PMAC64 is
selected.

I think this will make install images work in the G5 Macs.

Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/macintosh/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.9.4

Comments

Benjamin Herrenschmidt July 14, 2017, 10:53 p.m. UTC | #1 
On Fri, 2017-07-14 at 13:46 +0200, Linus Walleij wrote:
> I have this pretty nasty problem when trying to boot up a fresh

> openSuSE DVD on a PowerMac G5: the kernel by default does not have

> CONFIG_WINDFARM_PM72 enabled, with the effect that the cooling

> is not functioning.

> 

> The BIOS on the PowerMac G5 reacts to this by, after a grace

> period when the BIOS has waited for the OS to take over, increasing

> the fan speeds so it sounds like an airplane is in the room, and

> after another grace period simply cutting the power to the

> machine. This is done not beacuse the cooling is not working, but

> because the BIOS is not recieving handover of cooling from the

> OS, so it panics and give up. The problem has been reported by

> Linux users online.


It's not actually the BIOS but the fan controller HW who does that.

> Needless to say, this makes it impossible to install the OS

> before the machine turns itself off.

> 

> The g5_defconfig looks like this:

> CONFIG_PMAC_SMU=y

> CONFIG_WINDFARM=y

> CONFIG_WINDFARM_PM81=y

> CONFIG_WINDFARM_PM91=y

> CONFIG_WINDFARM_PM112=y

> CONFIG_WINDFARM_PM121=y

> 

> Notably PM72 is missing, making the PowerMac G5 fail.

> 

> The defconfig is not the right place to do this: it should be

> done by default when selecting Mac support for PPC/PPC64 and

> especially for the Macs CPUfreq driver. We select SMU by default

> for PPC_PMAC64, WINDFARM by default on PPC_PMAC and all the

> WINDFARM thermal managers by default if CPU_FREQ_PMAC64 is

> selected.

> 

> I think this will make install images work in the G5 Macs.


Why is it not the job of the defconfig ? I was under the impression
that just "selecting" like this was frowned upon ? I don't care much
either way mind you, I'll let Michael decide what he wants to do. 

> Cc: stable@vger.kernel.org

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  drivers/macintosh/Kconfig | 6 ++++++

>  1 file changed, 6 insertions(+)

> 

> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig

> index 97a420c11eed..d7186d8f30a9 100644

> --- a/drivers/macintosh/Kconfig

> +++ b/drivers/macintosh/Kconfig

> @@ -101,6 +101,7 @@ config ADB_PMU_LED_DISK

>  config PMAC_SMU

>  	bool "Support for SMU  based PowerMacs"

>  	depends on PPC_PMAC64

> +	default PPC_PMAC64

>  	help

>  	  This option adds support for the newer G5 iMacs and PowerMacs based

>  	  on the "SMU" system control chip which replaces the old PMU.

> @@ -194,11 +195,13 @@ config THERM_ADT746X

>  config WINDFARM

>  	tristate "New PowerMac thermal control infrastructure"

>  	depends on PPC

> +	default PPC_PMAC

>  

>  config WINDFARM_PM81

>  	tristate "Support for thermal management on iMac G5"

>  	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && PMAC_SMU

>  	select I2C_POWERMAC

> +	default CPU_FREQ_PMAC64

>  	help

>  	  This driver provides thermal control for the iMacG5

>  

> @@ -206,6 +209,7 @@ config WINDFARM_PM72

>  	tristate "Support for thermal management on PowerMac G5 (AGP)"

>  	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && ADB_PMU

>  	select I2C_POWERMAC

> +	default CPU_FREQ_PMAC64

>  	help

>  	  This driver provides thermal control for the PowerMac G5

>  	  "AGP" variants (PowerMac 7,2 and 7,3)

> @@ -214,6 +218,7 @@ config WINDFARM_RM31

>  	tristate "Support for thermal management on Xserve G5"

>  	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && ADB_PMU

>  	select I2C_POWERMAC

> +	default CPU_FREQ_PMAC64

>  	help

>  	  This driver provides thermal control for the Xserve G5

>  	  (RackMac3,1)

> @@ -222,6 +227,7 @@ config WINDFARM_PM91

>  	tristate "Support for thermal management on PowerMac9,1"

>  	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && PMAC_SMU

>  	select I2C_POWERMAC

> +	default CPU_FREQ_PMAC64

>  	help

>  	  This driver provides thermal control for the PowerMac9,1

>            which is the recent (SMU based) single CPU desktop G5
Linus Walleij July 15, 2017, 11:45 a.m. UTC | #2 
On Sat, Jul 15, 2017 at 12:53 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2017-07-14 at 13:46 +0200, Linus Walleij wrote:

>> I have this pretty nasty problem when trying to boot up a fresh

>> openSuSE DVD on a PowerMac G5: the kernel by default does not have

>> CONFIG_WINDFARM_PM72 enabled, with the effect that the cooling

>> is not functioning.

>>

>> The BIOS on the PowerMac G5 reacts to this by, after a grace

>> period when the BIOS has waited for the OS to take over, increasing

>> the fan speeds so it sounds like an airplane is in the room, and

>> after another grace period simply cutting the power to the

>> machine. This is done not beacuse the cooling is not working, but

>> because the BIOS is not recieving handover of cooling from the

>> OS, so it panics and give up. The problem has been reported by

>> Linux users online.

>

> It's not actually the BIOS but the fan controller HW who does that.


OK then :)

Are you running this hardware BTW?

>> I think this will make install images work in the G5 Macs.

>

> Why is it not the job of the defconfig ? I was under the impression

> that just "selecting" like this was frowned upon ? I don't care much

> either way mind you, I'll let Michael decide what he wants to do.


I think nobody is happy with Kconfig really.

This relates to a recent Kconfig unmanageability discussion at
ksummit-discuss I'd say. The situation is not great, it is very hard
to make working configs and this patch is meant to help with that
using the crude tools we have.

I prefer to just think about making it easy to do the right thing.
And making Kconfig do the right thing without human
intervention, because humans just screw everything up.
As is proven by the openSuSE install media situation.

They simply have no clue what to enable to create an installable
powermac media, even though they obviously know their way
around ppc64, it's just too hard to get the Kconfig right.

So if I patch arch/arm/configs/g5_defconfig it starts working for me,
true.

But I would not be surprised if the distros just forget to sync their
configs with g5_defconfig so it doesn't help at all but instead I have
to go and poke everyone and their dog about it.

Debian obviously had this enabled in *their* defconfig, because that
installed just fine with their Jessue distro. They have since dropped
support for ppc64 yay.

An alternative option is to go in and patch
drivers/cpufreq/Kconfig.powerpc like that:

config CPU_FREQ_PMAC64
        bool "Support for some Apple G5s"
        depends on PPC_PMAC && PPC64
+        select WINDFARM
+        select WINDFARM_PM81
+        select WINDFARM_PM72
+        select WINDFARM_RM31
+        select WINDFARM_PM91
+        select WINDFARM_PM112
+        select WINDFARM_PM121

If that is preferred?

Yours,
Linus Walleij
Benjamin Herrenschmidt July 15, 2017, 9:56 p.m. UTC | #3 
On Sat, 2017-07-15 at 13:45 +0200, Linus Walleij wrote:
> 

> > It's not actually the BIOS but the fan controller HW who does that.

> 

> OK then :)

> 

> Are you running this hardware BTW?


A pm_72 ? No but I might still be able to find one somewhere in the lab
if needed (not sure).

> > > I think this will make install images work in the G5 Macs.

> > 

> > Why is it not the job of the defconfig ? I was under the impression

> > that just "selecting" like this was frowned upon ? I don't care much

> > either way mind you, I'll let Michael decide what he wants to do.

> 

> I think nobody is happy with Kconfig really.

> 

> This relates to a recent Kconfig unmanageability discussion at

> ksummit-discuss I'd say. The situation is not great, it is very hard

> to make working configs and this patch is meant to help with that

> using the crude tools we have.


Yup.

> I prefer to just think about making it easy to do the right thing.

> And making Kconfig do the right thing without human

> intervention, because humans just screw everything up.

> As is proven by the openSuSE install media situation.

> 

> They simply have no clue what to enable to create an installable

> powermac media, even though they obviously know their way

> around ppc64, it's just too hard to get the Kconfig right.

> 

> So if I patch arch/arm/configs/g5_defconfig it starts working for me,

> true.

> 

> But I would not be surprised if the distros just forget to sync their

> configs with g5_defconfig so it doesn't help at all but instead I have

> to go and poke everyone and their dog about it.

> 

> Debian obviously had this enabled in *their* defconfig, because that

> installed just fine with their Jessue distro. They have since dropped

> support for ppc64 yay.

> 

> An alternative option is to go in and patch

> drivers/cpufreq/Kconfig.powerpc like that:

> 

> config CPU_FREQ_PMAC64

>         bool "Support for some Apple G5s"

>         depends on PPC_PMAC && PPC64

> +        select WINDFARM

> +        select WINDFARM_PM81

> +        select WINDFARM_PM72

> +        select WINDFARM_RM31

> +        select WINDFARM_PM91

> +        select WINDFARM_PM112

> +        select WINDFARM_PM121

> 

> If that is preferred?


Not really. I don't like also how we end up selecting etc... based on
CPUFREQ... I wouldn't mind select'ing cpufreq itself but then we end up
having to pull i2c etc..

In the end, your defaults are probably be way to go but make them just
default y, the dependency on CPUFREQ should be sufficient. Or will that
screw up with modules ?

Cheers,
Ben.
Michael Ellerman July 17, 2017, 2:02 a.m. UTC | #4 
Linus Walleij <linus.walleij@linaro.org> writes:

> I have this pretty nasty problem when trying to boot up a fresh

> openSuSE DVD on a PowerMac G5: the kernel by default does not have

> CONFIG_WINDFARM_PM72 enabled, with the effect that the cooling

> is not functioning.

>

> The BIOS on the PowerMac G5 reacts to this by, after a grace

> period when the BIOS has waited for the OS to take over, increasing

> the fan speeds so it sounds like an airplane is in the room, and

> after another grace period simply cutting the power to the

> machine. This is done not beacuse the cooling is not working, but

> because the BIOS is not recieving handover of cooling from the

> OS, so it panics and give up. The problem has been reported by

> Linux users online.

>

> Needless to say, this makes it impossible to install the OS

> before the machine turns itself off.

>

> The g5_defconfig looks like this:

> CONFIG_PMAC_SMU=y

> CONFIG_WINDFARM=y

> CONFIG_WINDFARM_PM81=y

> CONFIG_WINDFARM_PM91=y

> CONFIG_WINDFARM_PM112=y

> CONFIG_WINDFARM_PM121=y

>

> Notably PM72 is missing, making the PowerMac G5 fail.

>

> The defconfig is not the right place to do this: it should be

> done by default when selecting Mac support for PPC/PPC64 and

> especially for the Macs CPUfreq driver. We select SMU by default

> for PPC_PMAC64, WINDFARM by default on PPC_PMAC and all the

> WINDFARM thermal managers by default if CPU_FREQ_PMAC64 is

> selected.


I agree with the intent of your patch, but I'm not sure I like the
implementation.

> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig

> index 97a420c11eed..d7186d8f30a9 100644

> --- a/drivers/macintosh/Kconfig

> +++ b/drivers/macintosh/Kconfig

> @@ -101,6 +101,7 @@ config ADB_PMU_LED_DISK

>  config PMAC_SMU

>  	bool "Support for SMU  based PowerMacs"

>  	depends on PPC_PMAC64

> +	default PPC_PMAC64


Defaulting to one of your dependencies is exactly the same as defaulting
to yes (unless it's a tristate).

I think it's fine to make this default y, it has the dependency on the
platform so it won't appear for most users.

> @@ -194,11 +195,13 @@ config THERM_ADT746X

>  config WINDFARM

>  	tristate "New PowerMac thermal control infrastructure"

>  	depends on PPC

> +	default PPC_PMAC

>  

>  config WINDFARM_PM81

>  	tristate "Support for thermal management on iMac G5"

>  	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && PMAC_SMU

>  	select I2C_POWERMAC

> +	default CPU_FREQ_PMAC64

>  	help

>  	  This driver provides thermal control for the iMacG5


These I think should all just be 'default m'.

cheers
diff mbox

Patch

diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
index 97a420c11eed..d7186d8f30a9 100644
--- a/drivers/macintosh/Kconfig
+++ b/drivers/macintosh/Kconfig
@@ -101,6 +101,7 @@  config ADB_PMU_LED_DISK
 config PMAC_SMU
 	bool "Support for SMU  based PowerMacs"
 	depends on PPC_PMAC64
+	default PPC_PMAC64
 	help
 	  This option adds support for the newer G5 iMacs and PowerMacs based
 	  on the "SMU" system control chip which replaces the old PMU.
@@ -194,11 +195,13 @@  config THERM_ADT746X
 config WINDFARM
 	tristate "New PowerMac thermal control infrastructure"
 	depends on PPC
+	default PPC_PMAC
 
 config WINDFARM_PM81
 	tristate "Support for thermal management on iMac G5"
 	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && PMAC_SMU
 	select I2C_POWERMAC
+	default CPU_FREQ_PMAC64
 	help
 	  This driver provides thermal control for the iMacG5
 
@@ -206,6 +209,7 @@  config WINDFARM_PM72
 	tristate "Support for thermal management on PowerMac G5 (AGP)"
 	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && ADB_PMU
 	select I2C_POWERMAC
+	default CPU_FREQ_PMAC64
 	help
 	  This driver provides thermal control for the PowerMac G5
 	  "AGP" variants (PowerMac 7,2 and 7,3)
@@ -214,6 +218,7 @@  config WINDFARM_RM31
 	tristate "Support for thermal management on Xserve G5"
 	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && ADB_PMU
 	select I2C_POWERMAC
+	default CPU_FREQ_PMAC64
 	help
 	  This driver provides thermal control for the Xserve G5
 	  (RackMac3,1)
@@ -222,6 +227,7 @@  config WINDFARM_PM91
 	tristate "Support for thermal management on PowerMac9,1"
 	depends on WINDFARM && I2C && CPU_FREQ_PMAC64 && PMAC_SMU
 	select I2C_POWERMAC
+	default CPU_FREQ_PMAC64
 	help
 	  This driver provides thermal control for the PowerMac9,1
           which is the recent (SMU based) single CPU desktop G5