Change in coreboot[master]: [WIP] soc/amd/common: factor out SMU code from Picasso

[Felix Held (Code Review)]

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

[WIP] soc/amd/common: factor out SMU code from Picasso

The SMU mailbox access code from Picasso can be reused in the next
generation, so factor out the code to soc/amd/common/block/smu.

The message ID argument of the send_smu_message function is changed from
the SoC specific smu_message_id enum to an uint32_t to avoid including
the SoC-specific header file in the common SMU header file.

The patch is still WIP, since I'm still undecided if it's a bad idea to
include the SoC's pci_devs.h in the common SMU code for the definition
of SOC_GNB_DEV. It's also still untested on google/zork.

Change-Id: Ibaf5b91ab35428e4c771e7163c6e0c4fc50371e7
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/smu.h
A src/soc/amd/common/block/smu/Kconfig
A src/soc/amd/common/block/smu/Makefile.inc
A src/soc/amd/common/block/smu/smu.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/smu.h
M src/soc/amd/picasso/smu.c
7 files changed, 117 insertions(+), 99 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/47483/1

diff --git a/src/soc/amd/common/block/include/amdblocks/smu.h b/src/soc/amd/common/block/include/amdblocks/smu.h
new file mode 100644
index 0000000..ffc0d7a
--- /dev/null
+++ b/src/soc/amd/common/block/include/amdblocks/smu.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __AMDBLOCKS_SMU_H__
+#define __AMDBLOCKS_SMU_H__
+
+#include <types.h>
+
+/* SMU registers accessed indirectly using an index/data pair in D0F00 config space */
+#define SMU_INDEX_ADDR		0xb8 /* 32 bit */
+#define SMU_DATA_ADDR		0xbc /* 32 bit */
+
+#define REG_ADDR_MESG_ID	0x3b10528
+#define REG_ADDR_MESG_RESP	0x3b10564
+#define REG_ADDR_MESG_ARGS_BASE	0x3b10998
+
+/* Argument 0-5 indexed locations are contiguous */
+#define SMU_NUM_ARGS		6
+#define REG_ADDR_MESG_ARG(x)	(REG_ADDR_MESG_ARGS_BASE + ((x) * sizeof(uint32_t)))
+
+struct smu_payload {
+	uint32_t msg[SMU_NUM_ARGS];
+};
+
+/*
+ * Send a message and bi-directional payload to the SMU.  SMU response, if
+ * any, is returned via arg.  Returns 0 if success or -1 on failure.
+ */
+enum cb_err send_smu_message(uint32_t message_id, struct smu_payload *arg);
+
+#endif /* __AMDBLOCKS_SMU_H__ */
diff --git a/src/soc/amd/common/block/smu/Kconfig b/src/soc/amd/common/block/smu/Kconfig
new file mode 100644
index 0000000..60c231f
--- /dev/null
+++ b/src/soc/amd/common/block/smu/Kconfig
@@ -0,0 +1,5 @@
+config SOC_AMD_COMMON_BLOCK_SMU
+	bool
+	default n
+	help
+	  Select this option to add functions to communicate with the SMU to the build.
diff --git a/src/soc/amd/common/block/smu/Makefile.inc b/src/soc/amd/common/block/smu/Makefile.inc
new file mode 100644
index 0000000..2afd81a
--- /dev/null
+++ b/src/soc/amd/common/block/smu/Makefile.inc
@@ -0,0 +1 @@
+smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_SMU) += smu.c
diff --git a/src/soc/amd/common/block/smu/smu.c b/src/soc/amd/common/block/smu/smu.c
new file mode 100644
index 0000000..0d7b82b
--- /dev/null
+++ b/src/soc/amd/common/block/smu/smu.c
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <timer.h>
+#include <console/console.h>
+#include <device/pci_ops.h>
+#include <soc/pci_devs.h>
+#include <amdblocks/smu.h>
+#include <types.h>
+
+static uint32_t smu_read32(uint32_t reg)
+{
+	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
+	return pci_read_config32(SOC_GNB_DEV, SMU_DATA_ADDR);
+}
+
+static void smu_write32(uint32_t reg, uint32_t val)
+{
+	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
+	pci_write_config32(SOC_GNB_DEV, SMU_DATA_ADDR, val);
+}
+
+#define SMU_MESG_RESP_TIMEOUT	0x00
+#define SMU_MESG_RESP_OK	0x01
+
+/* returns SMU_MESG_RESP_OK, SMU_MESG_RESP_TIMEOUT or a negative number */
+static int32_t smu_poll_response(bool print_command_duration)
+{
+	struct stopwatch sw;
+	const long timeout_ms = 10 * MSECS_PER_SEC;
+	int32_t result;
+
+	stopwatch_init_msecs_expire(&sw, timeout_ms);
+
+	do {
+		result = smu_read32(REG_ADDR_MESG_RESP);
+		if (result) {
+			if (print_command_duration)
+				printk(BIOS_SPEW, "SMU command consumed %ld usecs\n",
+					stopwatch_duration_usecs(&sw));
+			return result;
+		}
+	} while (!stopwatch_expired(&sw));
+
+	printk(BIOS_ERR, "Error: timeout sending SMU message\n");
+	return SMU_MESG_RESP_TIMEOUT;
+}
+
+/*
+ * Send a message and bi-directional payload to the SMU. SMU response, if any, is returned via
+ * arg.
+ */
+enum cb_err send_smu_message(uint32_t message_id, struct smu_payload *arg)
+{
+	size_t i;
+
+	/* wait until SMU can process a new request; don't care if an old request failed */
+	if (smu_poll_response(false) == SMU_MESG_RESP_TIMEOUT)
+		return CB_ERR;
+
+	/* clear response register */
+	smu_write32(REG_ADDR_MESG_RESP, 0);
+
+	/* populate arguments */
+	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
+		smu_write32(REG_ADDR_MESG_ARG(i), arg->msg[i]);
+
+	/* send message to SMU */
+	smu_write32(REG_ADDR_MESG_ID, message_id);
+
+	/* wait until SMU has processed the message and check if it was successful */
+	if (smu_poll_response(true) != SMU_MESG_RESP_OK)
+		return CB_ERR;
+
+	/* copy returned values */
+	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
+		arg->msg[i] = smu_read32(REG_ADDR_MESG_ARG(i));
+
+	return CB_SUCCESS;
+}
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 5ae0a2a..9ebc651 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -41,6 +41,7 @@
    select SOC_AMD_COMMON_BLOCK_HDA
    select SOC_AMD_COMMON_BLOCK_SATA
    select SOC_AMD_COMMON_BLOCK_SMBUS
+	select SOC_AMD_COMMON_BLOCK_SMU
    select SOC_AMD_COMMON_BLOCK_PSP_GEN2
    select PROVIDES_ROM_SHARING
    select BOOT_DEVICE_SUPPORTS_WRITES if BOOT_DEVICE_SPI_FLASH
diff --git a/src/soc/amd/picasso/include/soc/smu.h b/src/soc/amd/picasso/include/soc/smu.h
index eb7573c..b00a941 100644
--- a/src/soc/amd/picasso/include/soc/smu.h
+++ b/src/soc/amd/picasso/include/soc/smu.h
@@ -3,34 +3,10 @@
 #ifndef __PICASSO_SMU_H__
 #define __PICASSO_SMU_H__

-#include <types.h>
-
-/* SMU registers accessed indirectly using an index/data pair in D0F00 config space */
-#define SMU_INDEX_ADDR		0xb8 /* 32 bit */
-#define SMU_DATA_ADDR		0xbc /* 32 bit */
-
-#define REG_ADDR_MESG_ID	0x3b10528
-#define REG_ADDR_MESG_RESP	0x3b10564
-#define REG_ADDR_MESG_ARGS_BASE	0x3b10998
-
-/* Argument 0-5 indexed locations are contiguous */
-#define SMU_NUM_ARGS		6
-#define REG_ADDR_MESG_ARG(x)	(REG_ADDR_MESG_ARGS_BASE + ((x) * sizeof(uint32_t)))
-
 enum smu_message_id {
    SMC_MSG_S3ENTRY = 0x0c,
 };

-struct smu_payload {
-	uint32_t msg[SMU_NUM_ARGS];
-};
-
-/*
- * Send a message and bi-directional payload to the SMU.  SMU response, if
- * any, is returned via arg.  Returns 0 if success or -1 on failure.
- */
-enum cb_err send_smu_message(enum smu_message_id id, struct smu_payload *arg);
-
 /*
  * Request the SMU put system into S3, S4, or S5.  On entry, SlpTyp determines
  * S-State and SlpTypeEn is clear.  Function does not return if successful.
diff --git a/src/soc/amd/picasso/smu.c b/src/soc/amd/picasso/smu.c
index 4a373ce..34be4e8 100644
--- a/src/soc/amd/picasso/smu.c
+++ b/src/soc/amd/picasso/smu.c
@@ -1,82 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-#include <timer.h>
 #include <console/console.h>
-#include <device/pci_ops.h>
-#include <soc/pci_devs.h>
+#include <amdblocks/smu.h>
 #include <soc/smu.h>
-#include <types.h>
-
-static uint32_t smu_read32(uint32_t reg)
-{
-	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
-	return pci_read_config32(SOC_GNB_DEV, SMU_DATA_ADDR);
-}
-
-static void smu_write32(uint32_t reg, uint32_t val)
-{
-	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
-	pci_write_config32(SOC_GNB_DEV, SMU_DATA_ADDR, val);
-}
-
-#define SMU_MESG_RESP_TIMEOUT	0x00
-#define SMU_MESG_RESP_OK	0x01
-
-/* returns SMU_MESG_RESP_OK, SMU_MESG_RESP_TIMEOUT or a negative number */
-static int32_t smu_poll_response(bool print_command_duration)
-{
-	struct stopwatch sw;
-	const long timeout_ms = 10 * MSECS_PER_SEC;
-	int32_t result;
-
-	stopwatch_init_msecs_expire(&sw, timeout_ms);
-
-	do {
-		result = smu_read32(REG_ADDR_MESG_RESP);
-		if (result) {
-			if (print_command_duration)
-				printk(BIOS_SPEW, "SMU command consumed %ld usecs\n",
-					stopwatch_duration_usecs(&sw));
-			return result;
-		}
-	} while (!stopwatch_expired(&sw));
-
-	printk(BIOS_ERR, "Error: timeout sending SMU message\n");
-	return SMU_MESG_RESP_TIMEOUT;
-}
-
-/*
- * Send a message and bi-directional payload to the SMU. SMU response, if any, is returned via
- * arg.
- */
-enum cb_err send_smu_message(enum smu_message_id id, struct smu_payload *arg)
-{
-	size_t i;
-
-	/* wait until SMU can process a new request; don't care if an old request failed */
-	if (smu_poll_response(false) == SMU_MESG_RESP_TIMEOUT)
-		return CB_ERR;
-
-	/* clear response register */
-	smu_write32(REG_ADDR_MESG_RESP, 0);
-
-	/* populate arguments */
-	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
-		smu_write32(REG_ADDR_MESG_ARG(i), arg->msg[i]);
-
-	/* send message to SMU */
-	smu_write32(REG_ADDR_MESG_ID, id);
-
-	/* wait until SMU has processed the message and check if it was successful */
-	if (smu_poll_response(true) != SMU_MESG_RESP_OK)
-		return CB_ERR;
-
-	/* copy returned values */
-	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
-		arg->msg[i] = smu_read32(REG_ADDR_MESG_ARG(i));
-
-	return CB_SUCCESS;
-}

/*
  * Request the SMU to put system into S3, S4, or S5. On entry, SlpTyp determines S-State and

[Felix Held (Code Review)]

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

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 1:

I'm not too happy about how I get the PCI device (that at least on the APUs will likely always be function 0 of device 0 on bus 0) that is used to access the SMU mailbox; if you have a better idea, please let me know

[Felix Held (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Marshall Dawson, Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

[WIP] soc/amd/common: factor out SMU code from Picasso

The SMU mailbox access code from Picasso can be reused in the next
generation, so factor out the code to soc/amd/common/block/smu.

The message ID argument of the send_smu_message function is changed from
the SoC specific smu_message_id enum to an uint32_t to avoid including
the SoC-specific header file in the common SMU header file.

The patch is still untested on google/zork.

Change-Id: Ibaf5b91ab35428e4c771e7163c6e0c4fc50371e7
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/smu.h
A src/soc/amd/common/block/smu/Kconfig
A src/soc/amd/common/block/smu/Makefile.inc
A src/soc/amd/common/block/smu/smu.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/smu.h
M src/soc/amd/picasso/smu.c
7 files changed, 122 insertions(+), 99 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/47483/2

[Jeremy Soller (Code Review)]

Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 2: Code-Review-1

The SMU code here may work on Picasso, and may work on Renoir. However, it is not common to the AM4 CPUs. Matisse, for example, has different values for the REG_ADDR_* constants and also supports 8 arguments instead of 6.

[Jeremy Soller (Code Review)]

Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 2:
> Patch Set 2: Code-Review-1
>
> The SMU code here may work on Picasso, and may work on Renoir. However, it is not common to the AM4 CPUs. Matisse, for example, has different values for the REG_ADDR_* constants and also supports 8 arguments instead of 6.

Perhaps use Kconfig's to define those values, or have them be in an include file from the picasso soc?

#include <soc/smu_defs.h>

struct smu_payload {
...

[Felix Held (Code Review)]

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

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2: Code-Review-1
> >
> > The SMU code here may work on Picasso, and may work on Renoir. However, it is not common to the AM4 CPUs. Matisse, for example, has different values for the REG_ADDR_* constants and also supports 8 arguments instead of 6.
>
> Perhaps use Kconfig's to define those values, or have them be in an include file from the picasso soc?
>
> #include <soc/smu_defs.h>
>
> struct smu_payload {
> ...

I included soc-specific header files in an earlier version of the patch in the common code, but found that to be not very elegant, but it seems that there's not really a way around that

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 2:

Is it valuable to further separate common/block/smu support into the supported generations then have soc/<name> select the right one?  (Not that I'm anticipating being able to open source much SMU code soon, but would like to plan for it.)

[Felix Held (Code Review)]

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

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Is it valuable to further separate common/block/smu support into the supported generations then have soc/<name> select the right one?  (Not that I'm anticipating being able to open source much SMU code soon, but would like to plan for it.)

Some other component in soc/amd/common is already including soc-specific headers, so I'll take that route

[Felix Held (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Marshall Dawson, Jeremy Soller, Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: [WIP] soc/amd/common: factor out SMU code from Picasso
......................................................................

[WIP] soc/amd/common: factor out SMU code from Picasso

The SMU mailbox access code from Picasso can be reused in the next
generation, so factor out the code to soc/amd/common/block/smu. Since
the mailbox register offsets in the indirect address space, the number
of arguments and the message IDs don't always match between different
devices, keep those in the soc-specific directories.

The patch is still untested on google/zork.

Change-Id: Ibaf5b91ab35428e4c771e7163c6e0c4fc50371e7
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/smu.h
A src/soc/amd/common/block/smu/Kconfig
A src/soc/amd/common/block/smu/Makefile.inc
A src/soc/amd/common/block/smu/smu.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/smu.h
M src/soc/amd/picasso/smu.c
7 files changed, 120 insertions(+), 95 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/47483/3

Change in coreboot[master]: soc/amd/common: factor out SMU code from Picasso

[Felix Held (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Marshall Dawson, Jeremy Soller, Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: soc/amd/common: factor out SMU code from Picasso
......................................................................

soc/amd/common: factor out SMU code from Picasso

The SMU mailbox access code from Picasso can be reused in the next
generation, so factor out the code to soc/amd/common/block/smu. Since
the mailbox register offsets in the indirect address space, the number
of arguments and the message IDs don't always match between different
devices, keep those in the soc-specific directories.

Change-Id: Ibaf5b91ab35428e4c771e7163c6e0c4fc50371e7
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/smu.h
A src/soc/amd/common/block/smu/Kconfig
A src/soc/amd/common/block/smu/Makefile.inc
A src/soc/amd/common/block/smu/smu.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/smu.h
M src/soc/amd/picasso/smu.c
7 files changed, 120 insertions(+), 95 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/47483/4

Change in coreboot[master]: soc/amd/common: factor out SMU code from Picasso

[Felix Held (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Marshall Dawson, Jeremy Soller, Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#5).

Change subject: soc/amd/common: factor out SMU code from Picasso
......................................................................

soc/amd/common: factor out SMU code from Picasso

The SMU mailbox access code from Picasso can be reused in the next
generation, so factor out the code to soc/amd/common/block/smu. Since
the mailbox register offsets in the indirect address space, the number
of arguments and the message IDs don't always match between different
devices, keep those in the soc-specific directories.

Change-Id: Ibaf5b91ab35428e4c771e7163c6e0c4fc50371e7
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/smu.h
A src/soc/amd/common/block/smu/Kconfig
A src/soc/amd/common/block/smu/Makefile.inc
A src/soc/amd/common/block/smu/smu.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/smu.h
M src/soc/amd/picasso/smu.c
7 files changed, 121 insertions(+), 96 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/47483/5

Change in coreboot[master]: soc/amd/common: factor out SMU code from Picasso

[Jeremy Soller (Code Review)]

Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 5: Code-Review+1

This looks good to me now

Change in coreboot[master]: soc/amd/common: factor out SMU code from Picasso

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: soc/amd/common: factor out SMU code from Picasso
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[master]: soc/amd/common: factor out SMU code from Picasso

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47483 )

Change subject: soc/amd/common: factor out SMU code from Picasso
......................................................................

soc/amd/common: factor out SMU code from Picasso

The SMU mailbox access code from Picasso can be reused in the next
generation, so factor out the code to soc/amd/common/block/smu. Since
the mailbox register offsets in the indirect address space, the number
of arguments and the message IDs don't always match between different
devices, keep those in the soc-specific directories.

Change-Id: Ibaf5b91ab35428e4c771e7163c6e0c4fc50371e7
Signed-off-by: Felix Held 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47483
Tested-by: build bot (Jenkins) 
Reviewed-by: Jeremy Soller 
Reviewed-by: Marshall Dawson 
---
A src/soc/amd/common/block/include/amdblocks/smu.h
A src/soc/amd/common/block/smu/Kconfig
A src/soc/amd/common/block/smu/Makefile.inc
A src/soc/amd/common/block/smu/smu.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/smu.h
M src/soc/amd/picasso/smu.c
7 files changed, 121 insertions(+), 96 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Marshall Dawson: Looks good to me, approved
  Jeremy Soller: Looks good to me, but someone else must approve

diff --git a/src/soc/amd/common/block/include/amdblocks/smu.h b/src/soc/amd/common/block/include/amdblocks/smu.h
new file mode 100644
index 0000000..57ef3b3
--- /dev/null
+++ b/src/soc/amd/common/block/include/amdblocks/smu.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __AMD_BLOCK_SMU_H__
+#define __AMD_BLOCK_SMU_H__
+
+#include <types.h>
+#include <soc/smu.h> /* SoC-dependent definitions for SMU access */
+
+/* SMU registers accessed indirectly using an index/data pair in D0F00 config space */
+#define SMU_INDEX_ADDR		0xb8 /* 32 bit */
+#define SMU_DATA_ADDR		0xbc /* 32 bit */
+
+/* Arguments indexed locations are contiguous; the number is SoC-dependent */
+#define REG_ADDR_MESG_ARG(x)	(REG_ADDR_MESG_ARGS_BASE + ((x) * sizeof(uint32_t)))
+
+struct smu_payload {
+	uint32_t msg[SMU_NUM_ARGS];
+};
+
+/*
+ * Send a message and bi-directional payload to the SMU. The SMU's response, if any, is
+ * returned via *arg. Returns CB_SUCCESS if success or CB_ERR on failure.
+ */
+enum cb_err send_smu_message(enum smu_message_id message_id, struct smu_payload *arg);
+
+#endif /* __AMD_BLOCK_SMU_H__ */
diff --git a/src/soc/amd/common/block/smu/Kconfig b/src/soc/amd/common/block/smu/Kconfig
new file mode 100644
index 0000000..60c231f
--- /dev/null
+++ b/src/soc/amd/common/block/smu/Kconfig
@@ -0,0 +1,5 @@
+config SOC_AMD_COMMON_BLOCK_SMU
+	bool
+	default n
+	help
+	  Select this option to add functions to communicate with the SMU to the build.
diff --git a/src/soc/amd/common/block/smu/Makefile.inc b/src/soc/amd/common/block/smu/Makefile.inc
new file mode 100644
index 0000000..2afd81a
--- /dev/null
+++ b/src/soc/amd/common/block/smu/Makefile.inc
@@ -0,0 +1 @@
+smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_SMU) += smu.c
diff --git a/src/soc/amd/common/block/smu/smu.c b/src/soc/amd/common/block/smu/smu.c
new file mode 100644
index 0000000..4f9c1d1
--- /dev/null
+++ b/src/soc/amd/common/block/smu/smu.c
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <timer.h>
+#include <console/console.h>
+#include <device/pci_ops.h>
+#include <amdblocks/smu.h>
+#include <soc/pci_devs.h>
+#include <soc/smu.h>
+#include <types.h>
+
+static uint32_t smu_read32(uint32_t reg)
+{
+	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
+	return pci_read_config32(SOC_GNB_DEV, SMU_DATA_ADDR);
+}
+
+static void smu_write32(uint32_t reg, uint32_t val)
+{
+	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
+	pci_write_config32(SOC_GNB_DEV, SMU_DATA_ADDR, val);
+}
+
+#define SMU_MESG_RESP_TIMEOUT	0x00
+#define SMU_MESG_RESP_OK	0x01
+
+/* returns SMU_MESG_RESP_OK, SMU_MESG_RESP_TIMEOUT or a negative number */
+static int32_t smu_poll_response(bool print_command_duration)
+{
+	struct stopwatch sw;
+	const long timeout_ms = 10 * MSECS_PER_SEC;
+	int32_t result;
+
+	stopwatch_init_msecs_expire(&sw, timeout_ms);
+
+	do {
+		result = smu_read32(REG_ADDR_MESG_RESP);
+		if (result) {
+			if (print_command_duration)
+				printk(BIOS_SPEW, "SMU command consumed %ld usecs\n",
+					stopwatch_duration_usecs(&sw));
+			return result;
+		}
+	} while (!stopwatch_expired(&sw));
+
+	printk(BIOS_ERR, "Error: timeout sending SMU message\n");
+	return SMU_MESG_RESP_TIMEOUT;
+}
+
+/*
+ * Send a message and bi-directional payload to the SMU. SMU response, if any, is returned via
+ * *arg.
+ */
+enum cb_err send_smu_message(enum smu_message_id message_id, struct smu_payload *arg)
+{
+	size_t i;
+
+	/* wait until SMU can process a new request; don't care if an old request failed */
+	if (smu_poll_response(false) == SMU_MESG_RESP_TIMEOUT)
+		return CB_ERR;
+
+	/* clear response register */
+	smu_write32(REG_ADDR_MESG_RESP, 0);
+
+	/* populate arguments */
+	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
+		smu_write32(REG_ADDR_MESG_ARG(i), arg->msg[i]);
+
+	/* send message to SMU */
+	smu_write32(REG_ADDR_MESG_ID, message_id);
+
+	/* wait until SMU has processed the message and check if it was successful */
+	if (smu_poll_response(true) != SMU_MESG_RESP_OK)
+		return CB_ERR;
+
+	/* copy returned values */
+	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
+		arg->msg[i] = smu_read32(REG_ADDR_MESG_ARG(i));
+
+	return CB_SUCCESS;
+}
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 5ae0a2a..9ebc651 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -41,6 +41,7 @@
    select SOC_AMD_COMMON_BLOCK_HDA
    select SOC_AMD_COMMON_BLOCK_SATA
    select SOC_AMD_COMMON_BLOCK_SMBUS
+	select SOC_AMD_COMMON_BLOCK_SMU
    select SOC_AMD_COMMON_BLOCK_PSP_GEN2
    select PROVIDES_ROM_SHARING
    select BOOT_DEVICE_SUPPORTS_WRITES if BOOT_DEVICE_SPI_FLASH
diff --git a/src/soc/amd/picasso/include/soc/smu.h b/src/soc/amd/picasso/include/soc/smu.h
index eb7573c..c402508 100644
--- a/src/soc/amd/picasso/include/soc/smu.h
+++ b/src/soc/amd/picasso/include/soc/smu.h
@@ -3,37 +3,23 @@
 #ifndef __PICASSO_SMU_H__
 #define __PICASSO_SMU_H__

-#include <types.h>
-
-/* SMU registers accessed indirectly using an index/data pair in D0F00 config space */
-#define SMU_INDEX_ADDR		0xb8 /* 32 bit */
-#define SMU_DATA_ADDR		0xbc /* 32 bit */
-
+/*
+ * SMU mailbox register offsets in indirect address space accessed by an index/data pair in
+ * D0F00 config space.
+ */
 #define REG_ADDR_MESG_ID	0x3b10528
 #define REG_ADDR_MESG_RESP	0x3b10564
 #define REG_ADDR_MESG_ARGS_BASE	0x3b10998

-/* Argument 0-5 indexed locations are contiguous */
 #define SMU_NUM_ARGS		6
-#define REG_ADDR_MESG_ARG(x)	(REG_ADDR_MESG_ARGS_BASE + ((x) * sizeof(uint32_t)))

enum smu_message_id {
    SMC_MSG_S3ENTRY = 0x0c,
 };

-struct smu_payload {
-	uint32_t msg[SMU_NUM_ARGS];
-};
-
 /*
- * Send a message and bi-directional payload to the SMU.  SMU response, if
- * any, is returned via arg.  Returns 0 if success or -1 on failure.
- */
-enum cb_err send_smu_message(enum smu_message_id id, struct smu_payload *arg);
-
-/*
- * Request the SMU put system into S3, S4, or S5.  On entry, SlpTyp determines
- * S-State and SlpTypeEn is clear.  Function does not return if successful.
+ * Request the SMU put system into S3, S4, or S5. On entry, SlpTyp determines S-State and
+ * SlpTypeEn gets set by the SMU. Function does not return if successful.
  */
 void smu_sx_entry(void);

diff --git a/src/soc/amd/picasso/smu.c b/src/soc/amd/picasso/smu.c
index 4a373ce..1496957 100644
--- a/src/soc/amd/picasso/smu.c
+++ b/src/soc/amd/picasso/smu.c
@@ -1,86 +1,12 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-#include <timer.h>
 #include <console/console.h>
-#include <device/pci_ops.h>
-#include <soc/pci_devs.h>
+#include <amdblocks/smu.h>
 #include <soc/smu.h>
-#include <types.h>
-
-static uint32_t smu_read32(uint32_t reg)
-{
-	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
-	return pci_read_config32(SOC_GNB_DEV, SMU_DATA_ADDR);
-}
-
-static void smu_write32(uint32_t reg, uint32_t val)
-{
-	pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, reg);
-	pci_write_config32(SOC_GNB_DEV, SMU_DATA_ADDR, val);
-}
-
-#define SMU_MESG_RESP_TIMEOUT	0x00
-#define SMU_MESG_RESP_OK	0x01
-
-/* returns SMU_MESG_RESP_OK, SMU_MESG_RESP_TIMEOUT or a negative number */
-static int32_t smu_poll_response(bool print_command_duration)
-{
-	struct stopwatch sw;
-	const long timeout_ms = 10 * MSECS_PER_SEC;
-	int32_t result;
-
-	stopwatch_init_msecs_expire(&sw, timeout_ms);
-
-	do {
-		result = smu_read32(REG_ADDR_MESG_RESP);
-		if (result) {
-			if (print_command_duration)
-				printk(BIOS_SPEW, "SMU command consumed %ld usecs\n",
-					stopwatch_duration_usecs(&sw));
-			return result;
-		}
-	} while (!stopwatch_expired(&sw));
-
-	printk(BIOS_ERR, "Error: timeout sending SMU message\n");
-	return SMU_MESG_RESP_TIMEOUT;
-}
-
-/*
- * Send a message and bi-directional payload to the SMU. SMU response, if any, is returned via
- * arg.
- */
-enum cb_err send_smu_message(enum smu_message_id id, struct smu_payload *arg)
-{
-	size_t i;
-
-	/* wait until SMU can process a new request; don't care if an old request failed */
-	if (smu_poll_response(false) == SMU_MESG_RESP_TIMEOUT)
-		return CB_ERR;
-
-	/* clear response register */
-	smu_write32(REG_ADDR_MESG_RESP, 0);
-
-	/* populate arguments */
-	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
-		smu_write32(REG_ADDR_MESG_ARG(i), arg->msg[i]);
-
-	/* send message to SMU */
-	smu_write32(REG_ADDR_MESG_ID, id);
-
-	/* wait until SMU has processed the message and check if it was successful */
-	if (smu_poll_response(true) != SMU_MESG_RESP_OK)
-		return CB_ERR;
-
-	/* copy returned values */
-	for (i = 0 ; i < SMU_NUM_ARGS ; i++)
-		arg->msg[i] = smu_read32(REG_ADDR_MESG_ARG(i));
-
-	return CB_SUCCESS;
-}

/*
  * Request the SMU to put system into S3, S4, or S5. On entry, SlpTyp determines S-State and
- * SlpTypeEn is clear. Function does not return if successful.
+ * SlpTypeEn gets set by the SMU. Function does not return if successful.
  */
 void smu_sx_entry(void)
 {