Change in coreboot[master]: soc/amd/common: Detect SOC before access ESPI register

[Bao Zheng (Code Review)]

Hello Zheng Bao,

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

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

to review the following change.

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

soc/amd/common: Detect SOC before access ESPI register

Cezanne doesn't have eSPIx00034 register define in PPR.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/espi_util.c
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/1

diff --git a/src/soc/amd/common/block/lpc/espi_util.c b/src/soc/amd/common/block/lpc/espi_util.c
index 0878fb7..ddeae98 100644
--- a/src/soc/amd/common/block/lpc/espi_util.c
+++ b/src/soc/amd/common/block/lpc/espi_util.c
@@ -824,6 +824,10 @@
 static void espi_setup_subtractive_decode(const struct espi_config *mb_cfg)
 {
    uint32_t global_ctrl_reg;
+
+	if (CONFIG(SOC_AMD_CEZANNE))
+		return;
+
    global_ctrl_reg = espi_read32(ESPI_GLOBAL_CONTROL_1);

if (mb_cfg->subtractive_decode) {

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48842 )

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48842/1/src/soc/amd/common/block/lp... 
File src/soc/amd/common/block/lpc/espi_util.c:

https://review.coreboot.org/c/coreboot/+/48842/1/src/soc/amd/common/block/lp... 
PS1, Line 828: SOC_AMD_CEZANNE
Please do not add SoC specific checks in common code. Instead this should be handled by adding a new Kconfig option that explains what the config is and why a certain SoC might select it.

[Felix Held (Code Review)]

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

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48842/1/src/soc/amd/common/block/lp... 
File src/soc/amd/common/block/lpc/espi_util.c:

https://review.coreboot.org/c/coreboot/+/48842/1/src/soc/amd/common/block/lp... 
PS1, Line 828: SOC_AMD_CEZANNE
> Please do not add SoC specific checks in common code. […]

only had a very brief look, but adding a Kconfig option in soc/amd/common/block/lpc/Kconfig like ESPI_SUPPORTS_SUBTRACTIVE_DECODE that gets selected by stoneyridge and picasso should probably be the way to go. might also be worth a look if LPC still supports subtractive decode; if that was also dropped there, dropping the ESPI_ from the kconfig symbol and also using it in the LPC code would be good

[Felix Held (Code Review)]

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

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48842/1/src/soc/amd/common/block/lp... 
File src/soc/amd/common/block/lpc/espi_util.c:

https://review.coreboot.org/c/coreboot/+/48842/1/src/soc/amd/common/block/lp... 
PS1, Line 828: SOC_AMD_CEZANNE
> only had a very brief look, but adding a Kconfig option in soc/amd/common/block/lpc/Kconfig like ESP […]

might also be worth a thought to move the check to espi_setup and only call espi_setup_subtractive_decode if the platform supports subtractive decodes

[Bao Zheng (Code Review)]

Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

soc/amd/common: Detect SOC before access ESPI register

Cezanne doesn't have eSPIx00034 register define in PPR.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/espi_util.c
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/23

[Bao Zheng (Code Review)]

Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

soc/amd/common: Detect SOC before access ESPI register

Cezanne doesn't have eSPIx00034 register define in PPR.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/espi_util.c
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/24

[Bao Zheng (Code Review)]

Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

soc/amd/common: Detect SOC before access ESPI register

Cezanne doesn't have eSPIx00034 register define in PPR.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/Kconfig
M src/soc/amd/common/block/lpc/espi_util.c
M src/soc/amd/picasso/Kconfig
3 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/25

[Bao Zheng (Code Review)]

Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd/common: Detect SOC before access ESPI register
......................................................................

soc/amd/common: Detect SOC before access ESPI register

Cezanne doesn't have eSPIx00034 register define in PPR.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/Kconfig
M src/soc/amd/common/block/lpc/espi_util.c
M src/soc/amd/picasso/Kconfig
3 files changed, 9 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/26

Change in coreboot[master]: soc/amd/common: Add an option to selete if suports ESPI sub decode

[Bao Zheng (Code Review)]

Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd/common: Add an option to selete if suports ESPI sub decode
......................................................................

soc/amd/common: Add an option to selete if suports ESPI sub decode

Cezanne doesn't have eSPIx00034 register define in PPR. Currently only
Picasso need this option.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/Kconfig
M src/soc/amd/common/block/lpc/espi_util.c
M src/soc/amd/picasso/Kconfig
3 files changed, 8 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/27

Change in coreboot[master]: soc/amd/: Add an option to select if SOC supports ESPI sub decode

[Bao Zheng (Code Review)]

Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd/: Add an option to select if SOC supports ESPI sub decode
......................................................................

soc/amd/: Add an option to select if SOC supports ESPI sub decode

Cezanne doesn't have eSPIx00034 register define in PPR. Currently only
Picasso need this option.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/Kconfig
M src/soc/amd/common/block/lpc/espi_util.c
M src/soc/amd/picasso/Kconfig
3 files changed, 8 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/28

Change in coreboot[master]: soc/amd/: Add an option to select if SOC supports ESPI sub decode

[Bao Zheng (Code Review)]

Attention is currently required from: Furquan Shaikh, Felix Held.
Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48842 )

Change subject: soc/amd/: Add an option to select if SOC supports ESPI sub decode
......................................................................

Patch Set 28:

(2 comments)

File src/soc/amd/common/block/lpc/espi_util.c:

https://review.coreboot.org/c/coreboot/+/48842/comment/5d6b7a13_d10d613d 
PS1, Line 828: SOC_AMD_CEZANNE
> might also be worth a thought to move the check to espi_setup and only call espi_setup_subtractive_d […]

Done

https://review.coreboot.org/c/coreboot/+/48842/comment/50e3da75_00c119bb 
PS1, Line 828: SOC_AMD_CEZANNE
> might also be worth a thought to move the check to espi_setup and only call espi_setup_subtractive_d […]

Done

Change in coreboot[master]: soc/amd/: Add an option to select if SOC supports ESPI sub decode

[Paul Menzel (Code Review)]

Attention is currently required from: Bao Zheng, Furquan Shaikh, Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48842 )

Change subject: soc/amd/: Add an option to select if SOC supports ESPI sub decode
......................................................................

Patch Set 28:

(1 comment)

Commit Message:

https://review.coreboot.org/c/coreboot/+/48842/comment/d09d6b76_a807852f 
PS28, Line 7: soc/amd/:
Please remove the trailing slash from the prefix.

Change in coreboot[master]: soc/amd/: Add an option to select if SOC supports ESPI sub decode

[Felix Held (Code Review)]

Attention is currently required from: Bao Zheng, Furquan Shaikh.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48842 )

Change subject: soc/amd/: Add an option to select if SOC supports ESPI sub decode
......................................................................

Patch Set 28: Code-Review+2

Change in coreboot[master]: soc/amd: Add an option to select if SOC supports ESPI sub decode

[Bao Zheng (Code Review)]

Attention is currently required from: Furquan Shaikh.
Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Zheng Bao, Felix Held,

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

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

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

Change subject: soc/amd: Add an option to select if SOC supports ESPI sub decode
......................................................................

soc/amd: Add an option to select if SOC supports ESPI sub decode

Cezanne doesn't have eSPIx00034 register define in PPR. Currently only
Picasso need this option.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
---
M src/soc/amd/common/block/lpc/Kconfig
M src/soc/amd/common/block/lpc/espi_util.c
M src/soc/amd/picasso/Kconfig
3 files changed, 8 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48842/29

Change in coreboot[master]: soc/amd: Add an option to select if SOC supports ESPI sub decode

[Bao Zheng (Code Review)]

Attention is currently required from: Furquan Shaikh, Paul Menzel.
Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48842 )

Change subject: soc/amd: Add an option to select if SOC supports ESPI sub decode
......................................................................

Patch Set 29:

(2 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/48842/comment/1ae8d2e8_ca33d97e 
PS28, Line 7: soc/amd/:
> Please remove the trailing slash from the prefix.

Done

Patchset:

PS29: 
Need to wait other general code about LPC, eSPI

Change in coreboot[master]: soc/amd: Add an option to select if SOC supports ESPI sub decode

[Felix Held (Code Review)]

Attention is currently required from: Bao Zheng, Furquan Shaikh, Paul Menzel.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48842 )

Change subject: soc/amd: Add an option to select if SOC supports ESPI sub decode
......................................................................

Patch Set 29: Code-Review+2

Change in coreboot[master]: soc/amd: Add an option to select if SOC supports ESPI sub decode

[Felix Held (Code Review)]

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

Change subject: soc/amd: Add an option to select if SOC supports ESPI sub decode
......................................................................

soc/amd: Add an option to select if SOC supports ESPI sub decode

Cezanne doesn't have eSPIx00034 register define in PPR. Currently only
Picasso need this option.

Change-Id: Icb8e8a1a59393849395125108bfaa884839ce10f
Signed-off-by: Zheng Bao 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48842
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
---
M src/soc/amd/common/block/lpc/Kconfig
M src/soc/amd/common/block/lpc/espi_util.c
M src/soc/amd/picasso/Kconfig
3 files changed, 8 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Held: Looks good to me, approved

diff --git a/src/soc/amd/common/block/lpc/Kconfig b/src/soc/amd/common/block/lpc/Kconfig
index 1ec8dd4..3aaccf3 100644
--- a/src/soc/amd/common/block/lpc/Kconfig
+++ b/src/soc/amd/common/block/lpc/Kconfig
@@ -24,3 +24,8 @@
    help
      Select this option if mainboard uses eSPI instead of LPC (if supported
      by platform).
+
+config SOC_AMD_COMMON_BLOCK_HAS_ESPI_SUB_DECODE
+	bool
+	depends on SOC_AMD_COMMON_BLOCK_HAS_ESPI
+	default n
diff --git a/src/soc/amd/common/block/lpc/espi_util.c b/src/soc/amd/common/block/lpc/espi_util.c
index 0878fb7..7ee5386 100644
--- a/src/soc/amd/common/block/lpc/espi_util.c
+++ b/src/soc/amd/common/block/lpc/espi_util.c
@@ -916,7 +916,8 @@
    }

/* Enable subtractive decode if configured */
-	espi_setup_subtractive_decode(cfg);
+	if (CONFIG(SOC_AMD_COMMON_BLOCK_HAS_ESPI_SUB_DECODE))
+		espi_setup_subtractive_decode(cfg);

return 0;
 }
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index da3903b..93d2ef8 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -29,6 +29,7 @@
    select SOC_AMD_COMMON
    select SOC_AMD_COMMON_BLOCK_NONCAR
    select SOC_AMD_COMMON_BLOCK_HAS_ESPI
+	select SOC_AMD_COMMON_BLOCK_HAS_ESPI_SUB_DECODE
    select SOC_AMD_COMMON_BLOCK_IOMMU
    select SOC_AMD_COMMON_BLOCK_ACPIMMIO
    select SOC_AMD_COMMON_BLOCK_BANKED_GPIOS