Change in coreboot[master]: soc/amd/picasso: Add helper functions for finding SOC type

[Felix Held (Code Review)]

Hello Raul Rangel, Martin Roth, Eric Peers,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/39886

to review the following change.

Change subject: soc/amd/picasso: Add helper functions for finding SOC type
......................................................................

soc/amd/picasso: Add helper functions for finding SOC type

We're running into more and more situations where we need to tell one
SOC type from another, and instead of rewriting them every time, just
add some helper functions to the picasso SOC directory.

Change-Id: I24b73145cdfa80c09fbe036d1fb6079696c6d013
Signed-off-by: Martin Roth 
Reviewed-on: https://chromium-review.googlesource.com/2051514
Reviewed-on: https://chromium-review.googlesource.com/2060904
Reviewed-on: https://chromium-review.googlesource.com/2060905
Reviewed-by: Martin Roth 
Reviewed-by: Eric Peers 
Reviewed-by: Raul E Rangel 
Signed-off-by: Felix Held 
---
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/include/soc/soc_util.h
A src/soc/amd/picasso/soc_util.c
3 files changed, 43 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/39886/1

diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 48f6507..6b32c6e 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -43,6 +43,7 @@
 romstage-y += tsc_freq.c
 romstage-y += southbridge.c
 romstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
+romstage-y += soc_util.c

verstage-y += gpio.c
 verstage-y += i2c.c
@@ -74,6 +75,7 @@
 ramstage-y += usb.c
 ramstage-y += tsc_freq.c
 ramstage-y += finalize.c
+ramstage-y += soc_util.c

all-y += cfg_util.c
 all-y += reset.c
diff --git a/src/soc/amd/picasso/include/soc/soc_util.h b/src/soc/amd/picasso/include/soc/soc_util.h
new file mode 100644
index 0000000..be05d9f
--- /dev/null
+++ b/src/soc/amd/picasso/include/soc/soc_util.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+int soc_is_pollock(void);
+int soc_is_dali(void);
+int soc_is_picasso(void);
+int soc_is_raven2(void);
+int soc_is_zen_plus(void);
diff --git a/src/soc/amd/picasso/soc_util.c b/src/soc/amd/picasso/soc_util.c
new file mode 100644
index 0000000..893ff25
--- /dev/null
+++ b/src/soc/amd/picasso/soc_util.c
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#include <arch/cpu.h>
+#include <soc/cpu.h>
+#include <soc/soc_util.h>
+
+int soc_is_pollock(void)
+{
+	return soc_is_zen_plus() && CONFIG(AMD_FT5);
+}
+
+int soc_is_dali(void)
+{
+	return soc_is_raven2() && CONFIG(AMD_FP5);
+}
+
+int soc_is_picasso(void)
+{
+	return soc_is_zen_plus() && CONFIG(AMD_FP5);
+}
+
+int soc_is_raven2(void)
+{
+	/* mask lower model number nibble and stepping */
+	return cpuid_eax(1) >> 8 == RAVEN2_CPUID >> 8;
+}
+
+int soc_is_zen_plus(void)
+{
+	/* mask lower model number nibble and stepping */
+	return cpuid_eax(1) >> 8 == PICASSO_CPUID >> 8;
+}

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39886 )

Change subject: soc/amd/picasso: Add helper functions for finding SOC type
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39886/1/src/soc/amd/picasso/soc_uti... 
File src/soc/amd/picasso/soc_util.c:

https://review.coreboot.org/c/coreboot/+/39886/1/src/soc/amd/picasso/soc_uti... 
PS1, Line 26: RAVEN2_CPUID
So internally we have a additional check for dali_3250U
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...

Are you going to add that in a followup?

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39886 )

Change subject: soc/amd/picasso: Add helper functions for finding SOC type
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39886/1/src/soc/amd/picasso/soc_uti... 
File src/soc/amd/picasso/soc_util.c:

https://review.coreboot.org/c/coreboot/+/39886/1/src/soc/amd/picasso/soc_uti... 
PS1, Line 26: RAVEN2_CPUID
> So internally we have a additional check for dali_3250U […]

i asked a question in the corresponding ticket, since it's not 100% clear to me if that was just a temporary workaround for some engineering sample or if that is for a chip that ends up being used in production units. so if that's necessary, i'll add that as a follow-up. didn't include that yet, since it seemed a bit odd to me

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39886 )

Change subject: soc/amd/picasso: Add helper functions for finding SOC type
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/39886/1/src/soc/amd/picasso/soc_uti... 
File src/soc/amd/picasso/soc_util.c:

https://review.coreboot.org/c/coreboot/+/39886/1/src/soc/amd/picasso/soc_uti... 
PS1, Line 26: RAVEN2_CPUID
> i asked a question in the corresponding ticket, since it's not 100% clear to me if that was just a t […]

Ack

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39886 )

Change subject: soc/amd/picasso: Add helper functions for finding SOC type
......................................................................

Patch Set 1: Code-Review+1

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39886 )

Change subject: soc/amd/picasso: Add helper functions for finding SOC type
......................................................................

soc/amd/picasso: Add helper functions for finding SOC type

We're running into more and more situations where we need to tell one
SOC type from another, and instead of rewriting them every time, just
add some helper functions to the picasso SOC directory.

Change-Id: I24b73145cdfa80c09fbe036d1fb6079696c6d013
Signed-off-by: Martin Roth 
Reviewed-on: https://chromium-review.googlesource.com/2051514
Reviewed-on: https://chromium-review.googlesource.com/2060904
Reviewed-on: https://chromium-review.googlesource.com/2060905
Reviewed-by: Martin Roth 
Reviewed-by: Eric Peers 
Reviewed-by: Raul E Rangel 
Signed-off-by: Felix Held 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39886
Tested-by: build bot (Jenkins) 
Reviewed-by: Raul Rangel 
Reviewed-by: Angel Pons 
---
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/include/soc/soc_util.h
A src/soc/amd/picasso/soc_util.c
3 files changed, 43 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Raul Rangel: Looks good to me, approved
  Angel Pons: Looks good to me, but someone else must approve

diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 48f6507..6b32c6e 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -43,6 +43,7 @@
 romstage-y += tsc_freq.c
 romstage-y += southbridge.c
 romstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
+romstage-y += soc_util.c

verstage-y += gpio.c
 verstage-y += i2c.c
@@ -74,6 +75,7 @@
 ramstage-y += usb.c
 ramstage-y += tsc_freq.c
 ramstage-y += finalize.c
+ramstage-y += soc_util.c

all-y += cfg_util.c
 all-y += reset.c
diff --git a/src/soc/amd/picasso/include/soc/soc_util.h b/src/soc/amd/picasso/include/soc/soc_util.h
new file mode 100644
index 0000000..be05d9f
--- /dev/null
+++ b/src/soc/amd/picasso/include/soc/soc_util.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+int soc_is_pollock(void);
+int soc_is_dali(void);
+int soc_is_picasso(void);
+int soc_is_raven2(void);
+int soc_is_zen_plus(void);
diff --git a/src/soc/amd/picasso/soc_util.c b/src/soc/amd/picasso/soc_util.c
new file mode 100644
index 0000000..893ff25
--- /dev/null
+++ b/src/soc/amd/picasso/soc_util.c
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#include <arch/cpu.h>
+#include <soc/cpu.h>
+#include <soc/soc_util.h>
+
+int soc_is_pollock(void)
+{
+	return soc_is_zen_plus() && CONFIG(AMD_FT5);
+}
+
+int soc_is_dali(void)
+{
+	return soc_is_raven2() && CONFIG(AMD_FP5);
+}
+
+int soc_is_picasso(void)
+{
+	return soc_is_zen_plus() && CONFIG(AMD_FP5);
+}
+
+int soc_is_raven2(void)
+{
+	/* mask lower model number nibble and stepping */
+	return cpuid_eax(1) >> 8 == RAVEN2_CPUID >> 8;
+}
+
+int soc_is_zen_plus(void)
+{
+	/* mask lower model number nibble and stepping */
+	return cpuid_eax(1) >> 8 == PICASSO_CPUID >> 8;
+}