Change in ...coreboot[master]: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h

[Marshall Dawson (Code Review)]

Hello Richard Spiegel, Martin Roth, Furquan Shaikh,

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

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

to review the following change.

Change subject: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h
......................................................................

soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h

Add the address and replace the hardcoded value in the ASL code.

Change-Id: If0b99de78d8c5948e2e5f2aa50dfc2efc1bd1ba1
Signed-off-by: Marshall Dawson 
---
M src/soc/amd/stoneyridge/acpi/sb_fch.asl
M src/soc/amd/stoneyridge/include/soc/iomap.h
2 files changed, 6 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32662/1

diff --git a/src/soc/amd/stoneyridge/acpi/sb_fch.asl b/src/soc/amd/stoneyridge/acpi/sb_fch.asl
index 4c1196d..e7975f8 100644
--- a/src/soc/amd/stoneyridge/acpi/sb_fch.asl
+++ b/src/soc/amd/stoneyridge/acpi/sb_fch.asl
@@ -22,7 +22,7 @@
    Name (_UID, 0x0)
    Name (_CRS, ResourceTemplate()
    {
-		Memory32Fixed (ReadWrite, 0xFEDC0000, 0x2000)
+		Memory32Fixed (ReadWrite, ALINK_AHB_ADDRESS, 0x2000)
    })

Method (_STA, 0x0, NotSerialized)
diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h
index 7762043..3856a22 100644
--- a/src/soc/amd/stoneyridge/include/soc/iomap.h
+++ b/src/soc/amd/stoneyridge/include/soc/iomap.h
@@ -25,10 +25,12 @@
 /* AcpiMmio blocks are at fixed offsets from FED8_0000h, enabled in PMx04[1] */
 #include <amdblocks/acpimmio_map.h>

+#define ALINK_AHB_ADDRESS		0xfedc0000
+
 /* I2C fixed address */
-#define I2C_BASE_ADDRESS	0xfedc2000
-#define I2C_DEVICE_SIZE		0x00001000
-#define I2C_DEVICE_COUNT	4
+#define I2C_BASE_ADDRESS		0xfedc2000
+#define I2C_DEVICE_SIZE			0x00001000
+#define I2C_DEVICE_COUNT		4

#if CONFIG(HPET_ADDRESS_OVERRIDE)
 #error HPET address override is not allowed and must be fixed at 0xfed00000

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32662 )

Change subject: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32662/1/src/soc/amd/stoneyridge/include/soc/... 
File src/soc/amd/stoneyridge/include/soc/iomap.h:

https://review.coreboot.org/#/c/32662/1/src/soc/amd/stoneyridge/include/soc/... 
PS1, Line 31: #define I2C_BASE_ADDRESS		0xfedc2000
            : #define I2C_DEVICE_SIZE			0x00001000
            : #define I2C_DEVICE_COUNT		4
            : 
            : 
I agree with alignment, but it does not belong to the commit (not mentioned on commit message). I do believe it should be a separate patch, I don't care with order (could be the very last patch, as a clean up).

[Marshall Dawson (Code Review)]

Hello Richard Spiegel, build bot (Jenkins), Martin Roth, Furquan Shaikh,

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

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

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

Change subject: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h
......................................................................

soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h

Add the address and replace the hardcoded value in the ASL code.

Change-Id: If0b99de78d8c5948e2e5f2aa50dfc2efc1bd1ba1
Signed-off-by: Marshall Dawson 
---
M src/soc/amd/stoneyridge/acpi/sb_fch.asl
M src/soc/amd/stoneyridge/include/soc/iomap.h
2 files changed, 3 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32662/2

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32662/1/src/soc/amd/stoneyridge/include/soc/... 
File src/soc/amd/stoneyridge/include/soc/iomap.h:

https://review.coreboot.org/#/c/32662/1/src/soc/amd/stoneyridge/include/soc/... 
PS1, Line 31: #define I2C_BASE_ADDRESS		0xfedc2000
            : #define I2C_DEVICE_SIZE			0x00001000
            : #define I2C_DEVICE_COUNT		4
            : 
            :
> I agree with alignment, but it does not belong to the commit (not mentioned on commit message). […]

Some amount of leniency is afforded to adjacent inconsequential changes, but fine... I intend to modify it later anyway.

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32662 )

Change subject: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h
......................................................................

Patch Set 2: Code-Review+2

[Furquan Shaikh (Code Review)]

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

Change subject: soc/amd/stoneyrige: Add ALink-AHB Bridge to iomap.h
......................................................................

Patch Set 4: Code-Review+2

Change in ...coreboot[master]: soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h

[Marshall Dawson (Code Review)]

Hello Richard Spiegel, build bot (Jenkins), Furquan Shaikh, Martin Roth,

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

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

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

Change subject: soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h
......................................................................

soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h

Add the address and replace the hardcoded value in the ASL code.

Change-Id: If0b99de78d8c5948e2e5f2aa50dfc2efc1bd1ba1
Signed-off-by: Marshall Dawson 
---
M src/soc/amd/stoneyridge/acpi/sb_fch.asl
M src/soc/amd/stoneyridge/include/soc/iomap.h
2 files changed, 3 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32662/5

Change in ...coreboot[master]: soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32662 )

Change subject: soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h
......................................................................

Patch Set 6: Code-Review+2

Change in ...coreboot[master]: soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32662 )

Change subject: soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h
......................................................................

soc/amd/stoneyridge: Add ALink-AHB Bridge to iomap.h

Add the address and replace the hardcoded value in the ASL code.

Change-Id: If0b99de78d8c5948e2e5f2aa50dfc2efc1bd1ba1
Signed-off-by: Marshall Dawson 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32662
Reviewed-by: Martin Roth 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/stoneyridge/acpi/sb_fch.asl
M src/soc/amd/stoneyridge/include/soc/iomap.h
2 files changed, 3 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved

diff --git a/src/soc/amd/stoneyridge/acpi/sb_fch.asl b/src/soc/amd/stoneyridge/acpi/sb_fch.asl
index 4c1196d..e7975f8 100644
--- a/src/soc/amd/stoneyridge/acpi/sb_fch.asl
+++ b/src/soc/amd/stoneyridge/acpi/sb_fch.asl
@@ -22,7 +22,7 @@
    Name (_UID, 0x0)
    Name (_CRS, ResourceTemplate()
    {
-		Memory32Fixed (ReadWrite, 0xFEDC0000, 0x2000)
+		Memory32Fixed (ReadWrite, ALINK_AHB_ADDRESS, 0x2000)
    })

Method (_STA, 0x0, NotSerialized)
diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h
index 1caef33..3856a22 100644
--- a/src/soc/amd/stoneyridge/include/soc/iomap.h
+++ b/src/soc/amd/stoneyridge/include/soc/iomap.h
@@ -25,6 +25,8 @@
 /* AcpiMmio blocks are at fixed offsets from FED8_0000h, enabled in PMx04[1] */
 #include <amdblocks/acpimmio_map.h>

+#define ALINK_AHB_ADDRESS		0xfedc0000
+
 /* I2C fixed address */
 #define I2C_BASE_ADDRESS		0xfedc2000
 #define I2C_DEVICE_SIZE			0x00001000