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@xxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx>

>  #endif
>  
>  	if (ppc_md.progress)

Sebastian