diff mbox series

[2/2] xscom: Parse P10 ec revision

Message ID 20220512042938.188001-3-joel@jms.id.au
State Superseded
Headers show
Series Report ec id for p10 | expand

Checks

Context Check Description
snowpatch_ozlabs/github-Docker_builds_and_checks fail check_build (ubuntu-rolling) failed at step Create Docker image.

Commit Message

Joel Stanley May 12, 2022, 4:29 a.m. UTC 
Use a look up table to support the p9, p10dd1 and p10dd2 conversions.

Running on a Rainier:

> [  268.267370706,6] CPU: P10 generation processor (max 4 threads/core)
> [  268.267372501,7] CPU: Boot CPU PIR is 0x0460 PVR is 0x00801200

> [  268.284420384,5] CHIP: Chip ID 0000 type: P10 DD2.02
> [  268.284464084,5] CHIP: Chip ID 0002 type: P10 DD2.02
> [  268.284500468,5] CHIP: Chip ID 0004 type: P10 DD2.02
> [  268.284538166,5] CHIP: Chip ID 0006 type: P10 DD2.02
> [  268.286317879,5] PLAT: Detected Rainier platform

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/xscom.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

Comments

Dan Horák May 12, 2022, 7:51 a.m. UTC | #1 
On Thu, 12 May 2022 13:59:38 +0930
Joel Stanley <joel@jms.id.au> wrote:

> Use a look up table to support the p9, p10dd1 and p10dd2 conversions.
> 
> Running on a Rainier:
> 
> > [  268.267370706,6] CPU: P10 generation processor (max 4 threads/core)
> > [  268.267372501,7] CPU: Boot CPU PIR is 0x0460 PVR is 0x00801200
> 
> > [  268.284420384,5] CHIP: Chip ID 0000 type: P10 DD2.02
> > [  268.284464084,5] CHIP: Chip ID 0002 type: P10 DD2.02
> > [  268.284500468,5] CHIP: Chip ID 0004 type: P10 DD2.02
> > [  268.284538166,5] CHIP: Chip ID 0006 type: P10 DD2.02
> > [  268.286317879,5] PLAT: Detected Rainier platform
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  hw/xscom.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/xscom.c b/hw/xscom.c
> index 862bd36ab75d..e40d69336ea6 100644
> --- a/hw/xscom.c
> +++ b/hw/xscom.c
> @@ -841,41 +841,42 @@ int64_t xscom_read_cfam_chipid(uint32_t partid, uint32_t *chip_id)
>  	return rc;
>  }
>  
> +/* The recipe comes from the p10_getecid hardware procedure */
>  static uint8_t xscom_get_ec_rev(struct proc_chip *chip)
>  {
>  	uint64_t ecid2 = 0;
> -	uint8_t rev;
> +	int8_t rev;
> +	const int8_t *table;
> +	/*                             0   1   2   3   4   5   6   7 */
> +	const int8_t p9table[8] =     {0,  1, -1,  2, -1, -1, -1,  3};
> +	const int8_t p10dd1table[8] = {0,  1,  2,  3, -1, -1,  4, -1};
> +	const int8_t p10dd2table[8] = {0,  2,  3,  4, -1, -1,  5, -1};
>  
>  	if (chip_quirk(QUIRK_MAMBO_CALLOUTS))
>  		return 0;
>  
>  	switch (proc_gen) {
>  	case proc_gen_p9:
> +		table = p9table;
> +		break;
> +	case proc_gen_p10:
> +		if (chip->ec_level < 0x20)
> +			table = p10dd1table;
> +		else
> +			table = p10dd2table;
>  		break;
>  	default:
>  		return 0;
>  	}
>  
>  	xscom_read(chip->id, 0x18002, &ecid2);
> -	switch((ecid2 >> 45) & 7) {
> -	case 0:
> -		rev = 0;
> -		break;
> -	case 1:
> -		rev = 1;
> -		break;
> -	case 3:
> -		rev = 2;
> -		break;
> -	case 7:
> -		rev = 3;
> -		break;
> -	default:
> -		rev = 0;
> -	}
> +
> +	rev = table[ecid2 >> 45 & 7];

personally I would prefer the parens there as used in the original
code, looks better readable to me


		Dan

> +	if (rev < 0)
> +		return 0;
>  
>  	prlog(PR_INFO,"P%d DD%i.%i%d detected\n",
> -			9,
> +			proc_gen == proc_gen_p9 ? 9 : 10,
>  			0xf & (chip->ec_level >> 4),
>  			chip->ec_level & 0xf,
>  			rev);
> -- 
> 2.35.1
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
Joel Stanley May 17, 2022, 8:40 a.m. UTC | #2 
On Thu, 12 May 2022 at 07:52, Dan Horák <dan@danny.cz> wrote:
>
> On Thu, 12 May 2022 13:59:38 +0930
> Joel Stanley <joel@jms.id.au> wrote:
>
> > Use a look up table to support the p9, p10dd1 and p10dd2 conversions.
> >
> > Running on a Rainier:
> >
> > > [  268.267370706,6] CPU: P10 generation processor (max 4 threads/core)
> > > [  268.267372501,7] CPU: Boot CPU PIR is 0x0460 PVR is 0x00801200
> >
> > > [  268.284420384,5] CHIP: Chip ID 0000 type: P10 DD2.02
> > > [  268.284464084,5] CHIP: Chip ID 0002 type: P10 DD2.02
> > > [  268.284500468,5] CHIP: Chip ID 0004 type: P10 DD2.02
> > > [  268.284538166,5] CHIP: Chip ID 0006 type: P10 DD2.02
> > > [  268.286317879,5] PLAT: Detected Rainier platform
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >  hw/xscom.c | 37 +++++++++++++++++++------------------
> >  1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/hw/xscom.c b/hw/xscom.c
> > index 862bd36ab75d..e40d69336ea6 100644
> > --- a/hw/xscom.c
> > +++ b/hw/xscom.c
> > @@ -841,41 +841,42 @@ int64_t xscom_read_cfam_chipid(uint32_t partid, uint32_t *chip_id)
> >       return rc;
> >  }
> >
> > +/* The recipe comes from the p10_getecid hardware procedure */
> >  static uint8_t xscom_get_ec_rev(struct proc_chip *chip)
> >  {
> >       uint64_t ecid2 = 0;
> > -     uint8_t rev;
> > +     int8_t rev;
> > +     const int8_t *table;
> > +     /*                             0   1   2   3   4   5   6   7 */
> > +     const int8_t p9table[8] =     {0,  1, -1,  2, -1, -1, -1,  3};
> > +     const int8_t p10dd1table[8] = {0,  1,  2,  3, -1, -1,  4, -1};
> > +     const int8_t p10dd2table[8] = {0,  2,  3,  4, -1, -1,  5, -1};
> >
> >       if (chip_quirk(QUIRK_MAMBO_CALLOUTS))
> >               return 0;
> >
> >       switch (proc_gen) {
> >       case proc_gen_p9:
> > +             table = p9table;
> > +             break;
> > +     case proc_gen_p10:
> > +             if (chip->ec_level < 0x20)
> > +                     table = p10dd1table;
> > +             else
> > +                     table = p10dd2table;
> >               break;
> >       default:
> >               return 0;
> >       }
> >
> >       xscom_read(chip->id, 0x18002, &ecid2);
> > -     switch((ecid2 >> 45) & 7) {
> > -     case 0:
> > -             rev = 0;
> > -             break;
> > -     case 1:
> > -             rev = 1;
> > -             break;
> > -     case 3:
> > -             rev = 2;
> > -             break;
> > -     case 7:
> > -             rev = 3;
> > -             break;
> > -     default:
> > -             rev = 0;
> > -     }
> > +
> > +     rev = table[ecid2 >> 45 & 7];
>
> personally I would prefer the parens there as used in the original
> code, looks better readable to me

Agreed, I didn't intend to change this. I'll send a v2.

>
>
>                 Dan
>
> > +     if (rev < 0)
> > +             return 0;
> >
> >       prlog(PR_INFO,"P%d DD%i.%i%d detected\n",
> > -                     9,
> > +                     proc_gen == proc_gen_p9 ? 9 : 10,
> >                       0xf & (chip->ec_level >> 4),
> >                       chip->ec_level & 0xf,
> >                       rev);
> > --
> > 2.35.1
> >
> > _______________________________________________
> > Skiboot mailing list
> > Skiboot@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/skiboot
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
diff mbox series

Patch

diff --git a/hw/xscom.c b/hw/xscom.c
index 862bd36ab75d..e40d69336ea6 100644
--- a/hw/xscom.c
+++ b/hw/xscom.c
@@ -841,41 +841,42 @@  int64_t xscom_read_cfam_chipid(uint32_t partid, uint32_t *chip_id)
 	return rc;
 }
 
+/* The recipe comes from the p10_getecid hardware procedure */
 static uint8_t xscom_get_ec_rev(struct proc_chip *chip)
 {
 	uint64_t ecid2 = 0;
-	uint8_t rev;
+	int8_t rev;
+	const int8_t *table;
+	/*                             0   1   2   3   4   5   6   7 */
+	const int8_t p9table[8] =     {0,  1, -1,  2, -1, -1, -1,  3};
+	const int8_t p10dd1table[8] = {0,  1,  2,  3, -1, -1,  4, -1};
+	const int8_t p10dd2table[8] = {0,  2,  3,  4, -1, -1,  5, -1};
 
 	if (chip_quirk(QUIRK_MAMBO_CALLOUTS))
 		return 0;
 
 	switch (proc_gen) {
 	case proc_gen_p9:
+		table = p9table;
+		break;
+	case proc_gen_p10:
+		if (chip->ec_level < 0x20)
+			table = p10dd1table;
+		else
+			table = p10dd2table;
 		break;
 	default:
 		return 0;
 	}
 
 	xscom_read(chip->id, 0x18002, &ecid2);
-	switch((ecid2 >> 45) & 7) {
-	case 0:
-		rev = 0;
-		break;
-	case 1:
-		rev = 1;
-		break;
-	case 3:
-		rev = 2;
-		break;
-	case 7:
-		rev = 3;
-		break;
-	default:
-		rev = 0;
-	}
+
+	rev = table[ecid2 >> 45 & 7];
+	if (rev < 0)
+		return 0;
 
 	prlog(PR_INFO,"P%d DD%i.%i%d detected\n",
-			9,
+			proc_gen == proc_gen_p9 ? 9 : 10,
 			0xf & (chip->ec_level >> 4),
 			chip->ec_level & 0xf,
 			rev);