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

[Mahesh J Salgaonkar]

On 2025-02-05 09:08:53 Wed, Reza Arbab wrote:
> 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

Those changes are from Aditya when he tried v1 series on hardware.

> 
> [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.

Agree. I will try adding "||" to other placesi too to make it consistent.

> 
> > --- 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"?

This is because Power brand wants to expport it as Power11.

Thanks,
-Mahesh.