[Skiboot] [PATCH v2 1/6] Initial Power11 enablement

Reza Arbab arbab at linux.ibm.com
Thu Feb 6 02:08:53 AEDT 2025


On Mon, Feb 03, 2025 at 11:46:48AM +0530, Mahesh Salgaonkar wrote:
>Detect Power11 PVR and use P10 code path.
>
>Signed-off-by: Aditya Gupta <adityag at linux.ibm.com>
>[adityag: Add Power11 chiptod device node]
>[adityag: Fix the proc_gen checks in pir_to_thread_id and bmc sensor]

If these are things Mahesh changed from Aditya's original patch, it 
should be

[mahesh: ...]

>--- a/core/affinity.c
>+++ b/core/affinity.c
>@@ -113,6 +113,8 @@ void add_core_associativity(struct cpu_thread *cpu)
> 		core_id = (cpu->pir >> 2) & 0x1f;
> 	else if (proc_gen == proc_gen_p10)
> 		core_id = (cpu->pir >> 2) & 0x1f;
>+	else if (proc_gen == proc_gen_p11)
>+		core_id = (cpu->pir >> 2) & 0x1f;
> 	else
> 		return;
>

In some places we add a completely identical "else if (proc_gen == 
proc_gen_p11)" arm, and in others we add "|| proc_gen == proc_gen_p11" 
to the p10 arm. I'm not sure which is better, or more readable, or if 
it's even worth changing anything, but it's inconsistent to the eye.

>--- a/core/cpu.c
>+++ b/core/cpu.c
>@@ -1084,6 +1092,14 @@ void init_boot_cpu(void)
> 		prlog(PR_INFO, "CPU: P10 generation processor"
> 		      " (max %d threads/core)\n", cpu_threads_max);
> 		break;
>+	case proc_gen_p11:
>+		if (is_fused_core(pvr))
>+			cpu_threads_max = 8;
>+		else
>+			cpu_threads_max = 4;
>+		prlog(PR_INFO, "CPU: Power11 generation processor"
>+		      " (max %d threads/core)\n", cpu_thread_count);

For the other cpus, we have
   prlog(PR_INFO, "CPU: P8 generation processor" ...
   prlog(PR_INFO, "CPU: P9 generation processor" ...
   prlog(PR_INFO, "CPU: P10 generation processor" ...

Is this intentional? Shouldn't it be "P11"?

-- 
Reza Arbab


More information about the Skiboot mailing list