[PATCH] powerpc/powermac: Always close the timebase-sync i2c bus

[PATCH] powerpc/powermac: Always close the timebase-sync i2c bus

[Daniel Palmer]

smp_core99_cpu_prepare() opens an i2c bus for "tb sync" and that is
closed again in smp_core99_cpu_online().

If a processor module is dysfunctional, not seated correctly or
otherwise fails to come online smp_core99_cpu_online() is not called
and the bus stays open.

On some G5s (tested PowerMac7,3) this i2c bus also has the CPU
temperature sensor and fan controller connected to it so not closing
the bus stops the kernel from making sure the working CPU doesn't
roast itself to death.

Rename smp_core99_cpu_online() to smp_core99_cpu_online_prepare_down()
and wire it up to get called when onlining fails as well so the
bus is closed and the working CPU stays working.

Fixes: 68e694dcef24 ("powerpc/powermac: Convert to hotplug state machine")
Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 arch/powerpc/platforms/powermac/smp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index 88e92af8acf9..e310b6fbbede 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -865,7 +865,7 @@ static int smp_core99_cpu_prepare(unsigned int cpu)
 	return 0;
 }
 
-static int smp_core99_cpu_online(unsigned int cpu)
+static int smp_core99_cpu_online_prepare_down(unsigned int cpu)
 {
 	/* Close i2c bus if it was used for tb sync */
 	if (pmac_tb_clock_chip_host && smp_core99_host_open) {
@@ -893,9 +893,9 @@ static void __init smp_core99_bringup_done(void)
 #ifdef CONFIG_HOTPLUG_CPU
 	cpuhp_setup_state_nocalls(CPUHP_POWERPC_PMAC_PREPARE,
 				  "powerpc/pmac:prepare", smp_core99_cpu_prepare,
-				  NULL);
+				  smp_core99_cpu_online_prepare_down);
 	cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "powerpc/pmac:online",
-				  smp_core99_cpu_online, NULL);
+				  smp_core99_cpu_online_prepare_down, NULL);
 #endif
 
 	if (ppc_md.progress)
-- 
2.55.0

Re: [PATCH] powerpc/powermac: Always close the timebase-sync i2c bus

[Sebastian Andrzej Siewior]

On 2026-10-31 08:24:18 [+0900], Daniel Palmer wrote:
> smp_core99_cpu_prepare() opens an i2c bus for "tb sync" and that is
> closed again in smp_core99_cpu_online().
> 
> If a processor module is dysfunctional, not seated correctly or
> otherwise fails to come online smp_core99_cpu_online() is not called
> and the bus stays open.
> 
> On some G5s (tested PowerMac7,3) this i2c bus also has the CPU
> temperature sensor and fan controller connected to it so not closing
> the bus stops the kernel from making sure the working CPU doesn't
> roast itself to death.
> 
> Rename smp_core99_cpu_online() to smp_core99_cpu_online_prepare_down()
> and wire it up to get called when onlining fails as well so the
> bus is closed and the working CPU stays working.

"and the I2C bus remains accessible by the i2c-driver."

The `smp_core99_host_open' makes sure it does not get closed twice so
that works.

> Fixes: 68e694dcef24 ("powerpc/powermac: Convert to hotplug state machine")

So the old CPU_UP_CANCELED case would have close it and I forgot to copy
that part.

> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  arch/powerpc/platforms/powermac/smp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
> index 88e92af8acf9..e310b6fbbede 100644
> --- a/arch/powerpc/platforms/powermac/smp.c
> +++ b/arch/powerpc/platforms/powermac/smp.c
> @@ -865,7 +865,7 @@ static int smp_core99_cpu_prepare(unsigned int cpu)
>  	return 0;
>  }
>  
> -static int smp_core99_cpu_online(unsigned int cpu)
> +static int smp_core99_cpu_online_prepare_down(unsigned int cpu)
>  {
>  	/* Close i2c bus if it was used for tb sync */
>  	if (pmac_tb_clock_chip_host && smp_core99_host_open) {
> @@ -893,9 +893,9 @@ static void __init smp_core99_bringup_done(void)
>  #ifdef CONFIG_HOTPLUG_CPU
>  	cpuhp_setup_state_nocalls(CPUHP_POWERPC_PMAC_PREPARE,
>  				  "powerpc/pmac:prepare", smp_core99_cpu_prepare,
> -				  NULL);
> +				  smp_core99_cpu_online_prepare_down);
>  	cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "powerpc/pmac:online",
> -				  smp_core99_cpu_online, NULL);
> +				  smp_core99_cpu_online_prepare_down, NULL);

I don't think it is needed to rename the function as such.  OpenBUS/
CloseBUS would be more matching than to the sate it runs in me think.
But it is not my call. For the "adding the function to the prepare
notifier" part:

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

>  #endif
>  
>  	if (ppc_md.progress)

Sebastian