Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32930
Change subject: soc/amd/common: Fix consistency in AcpiMmio arguments ......................................................................
soc/amd/common: Fix consistency in AcpiMmio arguments
Change all arguments named "offset" to "reg" to match the others.
These should have gone into change 69486cac7: Create AcpiMmio functionality from stoneyridge
Change-Id: Ifdd00d0a5d1e03bfa68a13eeece2d2cfd56aa39d Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/acpimmio/mmio_util.c M src/soc/amd/common/block/include/amdblocks/acpimmio.h 2 files changed, 37 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/32930/1
diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c index 7d4c4c5..281880c 100644 --- a/src/soc/amd/common/block/acpimmio/mmio_util.c +++ b/src/soc/amd/common/block/acpimmio/mmio_util.c @@ -69,34 +69,34 @@
/* smi read/write - access registers at 0xfed80200 */
-uint8_t smi_read8(uint8_t offset) +uint8_t smi_read8(uint8_t reg) { - return read8((void *)(ACPIMMIO_SMI_BASE + offset)); + return read8((void *)(ACPIMMIO_SMI_BASE + reg)); }
-uint16_t smi_read16(uint8_t offset) +uint16_t smi_read16(uint8_t reg) { - return read16((void *)(ACPIMMIO_SMI_BASE + offset)); + return read16((void *)(ACPIMMIO_SMI_BASE + reg)); }
-uint32_t smi_read32(uint8_t offset) +uint32_t smi_read32(uint8_t reg) { - return read32((void *)(ACPIMMIO_SMI_BASE + offset)); + return read32((void *)(ACPIMMIO_SMI_BASE + reg)); }
-void smi_write8(uint8_t offset, uint8_t value) +void smi_write8(uint8_t reg, uint8_t value) { - write8((void *)(ACPIMMIO_SMI_BASE + offset), value); + write8((void *)(ACPIMMIO_SMI_BASE + reg), value); }
-void smi_write16(uint8_t offset, uint16_t value) +void smi_write16(uint8_t reg, uint16_t value) { - write16((void *)(ACPIMMIO_SMI_BASE + offset), value); + write16((void *)(ACPIMMIO_SMI_BASE + reg), value); }
-void smi_write32(uint8_t offset, uint32_t value) +void smi_write32(uint8_t reg, uint32_t value) { - write32((void *)(ACPIMMIO_SMI_BASE + offset), value); + write32((void *)(ACPIMMIO_SMI_BASE + reg), value); }
/* pm read/write - access registers at 0xfed80300 */ @@ -135,45 +135,45 @@
/* biosram read/write - access registers at 0xfed80500 */
-uint8_t biosram_read8(uint8_t offset) +uint8_t biosram_read8(uint8_t reg) { - return read8((void *)(ACPIMMIO_BIOSRAM_BASE + offset)); + return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg)); }
-uint16_t biosram_read16(uint8_t offset) /* Must be 1 byte at a time */ +uint16_t biosram_read16(uint8_t reg) /* Must be 1 byte at a time */ { int i; uint16_t value = 0; for (i = sizeof(value) - 1 ; i >= 0 ; i--) - value = (value << 8) | biosram_read8(offset + i); + value = (value << 8) | biosram_read8(reg + i); return value; }
-uint32_t biosram_read32(uint8_t offset) +uint32_t biosram_read32(uint8_t reg) { - uint32_t value = biosram_read16(offset + sizeof(uint16_t)) << 16; - return value | biosram_read16(offset); + uint32_t value = biosram_read16(reg + sizeof(uint16_t)) << 16; + return value | biosram_read16(reg); }
-void biosram_write8(uint8_t offset, uint8_t value) +void biosram_write8(uint8_t reg, uint8_t value) { - write8((void *)(ACPIMMIO_BIOSRAM_BASE + offset), value); + write8((void *)(ACPIMMIO_BIOSRAM_BASE + reg), value); }
-void biosram_write16(uint8_t offset, uint16_t value) +void biosram_write16(uint8_t reg, uint16_t value) { int i; for (i = 0 ; i < sizeof(value) ; i++) { - biosram_write8(offset + i, value & 0xff); + biosram_write8(reg + i, value & 0xff); value >>= 8; } }
-void biosram_write32(uint8_t offset, uint32_t value) +void biosram_write32(uint8_t reg, uint32_t value) { int i; for (i = 0 ; i < sizeof(value) ; i++) { - biosram_write8(offset + i, value & 0xff); + biosram_write8(reg + i, value & 0xff); value >>= 8; } } diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 34e9ce9..c90c0c8 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -25,12 +25,12 @@ void pm_io_write8(uint8_t reg, uint8_t value); void pm_io_write16(uint8_t reg, uint16_t value); void pm_io_write32(uint8_t reg, uint32_t value); -uint8_t smi_read8(uint8_t offset); -uint16_t smi_read16(uint8_t offset); -uint32_t smi_read32(uint8_t offset); -void smi_write8(uint8_t offset, uint8_t value); -void smi_write16(uint8_t offset, uint16_t value); -void smi_write32(uint8_t offset, uint32_t value); +uint8_t smi_read8(uint8_t reg); +uint16_t smi_read16(uint8_t reg); +uint32_t smi_read32(uint8_t reg); +void smi_write8(uint8_t reg, uint8_t value); +void smi_write16(uint8_t reg, uint16_t value); +void smi_write32(uint8_t reg, uint32_t value); uint8_t pm_read8(uint8_t reg); uint16_t pm_read16(uint8_t reg); uint32_t pm_read32(uint8_t reg); @@ -43,12 +43,12 @@ void pm2_write8(uint8_t reg, uint8_t value); void pm2_write16(uint8_t reg, uint16_t value); void pm2_write32(uint8_t reg, uint32_t value); -uint8_t biosram_read8(uint8_t offset); -uint16_t biosram_read16(uint8_t offset); -uint32_t biosram_read32(uint8_t offset); -void biosram_write8(uint8_t offset, uint8_t value); -void biosram_write16(uint8_t offset, uint16_t value); -void biosram_write32(uint8_t offset, uint32_t value); +uint8_t biosram_read8(uint8_t reg); +uint16_t biosram_read16(uint8_t reg); +uint32_t biosram_read32(uint8_t reg); +void biosram_write8(uint8_t reg, uint8_t value); +void biosram_write16(uint8_t reg, uint16_t value); +void biosram_write32(uint8_t reg, uint32_t value); uint8_t acpi_read8(uint8_t reg); uint16_t acpi_read16(uint8_t reg); uint32_t acpi_read32(uint8_t reg);
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32930 )
Change subject: soc/amd/common: Fix consistency in AcpiMmio arguments ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32930/1/src/soc/amd/common/block/acpimmio/mm... File src/soc/amd/common/block/acpimmio/mmio_util.c:
https://review.coreboot.org/#/c/32930/1/src/soc/amd/common/block/acpimmio/mm... PS1, Line 147: for (i = sizeof(value) - 1 ; i >= 0 ; i--) : value = (value << 8) | biosram_read8(reg + i); While you are fixing things here, why not change this to: return biosram_read8(reg + sizeof(uint8_t)) << 8 + biosram_read8(reg);
https://review.coreboot.org/#/c/32930/1/src/soc/amd/common/block/acpimmio/mm... PS1, Line 166: for same here.
https://review.coreboot.org/#/c/32930/1/src/soc/amd/common/block/acpimmio/mm... PS1, Line 175: for and here.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32930 )
Change subject: soc/amd/common: Fix consistency in AcpiMmio arguments ......................................................................
Patch Set 1: Code-Review+2
I see you already addressed the comments in this CL in a later patch.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32930 )
Change subject: soc/amd/common: Fix consistency in AcpiMmio arguments ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32930 )
Change subject: soc/amd/common: Fix consistency in AcpiMmio arguments ......................................................................
soc/amd/common: Fix consistency in AcpiMmio arguments
Change all arguments named "offset" to "reg" to match the others.
These should have gone into change 69486cac7: Create AcpiMmio functionality from stoneyridge
Change-Id: Ifdd00d0a5d1e03bfa68a13eeece2d2cfd56aa39d Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32930 Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/common/block/acpimmio/mmio_util.c M src/soc/amd/common/block/include/amdblocks/acpimmio.h 2 files changed, 37 insertions(+), 37 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Richard Spiegel: Looks good to me, approved
diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c index 7d4c4c5..281880c 100644 --- a/src/soc/amd/common/block/acpimmio/mmio_util.c +++ b/src/soc/amd/common/block/acpimmio/mmio_util.c @@ -69,34 +69,34 @@
/* smi read/write - access registers at 0xfed80200 */
-uint8_t smi_read8(uint8_t offset) +uint8_t smi_read8(uint8_t reg) { - return read8((void *)(ACPIMMIO_SMI_BASE + offset)); + return read8((void *)(ACPIMMIO_SMI_BASE + reg)); }
-uint16_t smi_read16(uint8_t offset) +uint16_t smi_read16(uint8_t reg) { - return read16((void *)(ACPIMMIO_SMI_BASE + offset)); + return read16((void *)(ACPIMMIO_SMI_BASE + reg)); }
-uint32_t smi_read32(uint8_t offset) +uint32_t smi_read32(uint8_t reg) { - return read32((void *)(ACPIMMIO_SMI_BASE + offset)); + return read32((void *)(ACPIMMIO_SMI_BASE + reg)); }
-void smi_write8(uint8_t offset, uint8_t value) +void smi_write8(uint8_t reg, uint8_t value) { - write8((void *)(ACPIMMIO_SMI_BASE + offset), value); + write8((void *)(ACPIMMIO_SMI_BASE + reg), value); }
-void smi_write16(uint8_t offset, uint16_t value) +void smi_write16(uint8_t reg, uint16_t value) { - write16((void *)(ACPIMMIO_SMI_BASE + offset), value); + write16((void *)(ACPIMMIO_SMI_BASE + reg), value); }
-void smi_write32(uint8_t offset, uint32_t value) +void smi_write32(uint8_t reg, uint32_t value) { - write32((void *)(ACPIMMIO_SMI_BASE + offset), value); + write32((void *)(ACPIMMIO_SMI_BASE + reg), value); }
/* pm read/write - access registers at 0xfed80300 */ @@ -135,45 +135,45 @@
/* biosram read/write - access registers at 0xfed80500 */
-uint8_t biosram_read8(uint8_t offset) +uint8_t biosram_read8(uint8_t reg) { - return read8((void *)(ACPIMMIO_BIOSRAM_BASE + offset)); + return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg)); }
-uint16_t biosram_read16(uint8_t offset) /* Must be 1 byte at a time */ +uint16_t biosram_read16(uint8_t reg) /* Must be 1 byte at a time */ { int i; uint16_t value = 0; for (i = sizeof(value) - 1 ; i >= 0 ; i--) - value = (value << 8) | biosram_read8(offset + i); + value = (value << 8) | biosram_read8(reg + i); return value; }
-uint32_t biosram_read32(uint8_t offset) +uint32_t biosram_read32(uint8_t reg) { - uint32_t value = biosram_read16(offset + sizeof(uint16_t)) << 16; - return value | biosram_read16(offset); + uint32_t value = biosram_read16(reg + sizeof(uint16_t)) << 16; + return value | biosram_read16(reg); }
-void biosram_write8(uint8_t offset, uint8_t value) +void biosram_write8(uint8_t reg, uint8_t value) { - write8((void *)(ACPIMMIO_BIOSRAM_BASE + offset), value); + write8((void *)(ACPIMMIO_BIOSRAM_BASE + reg), value); }
-void biosram_write16(uint8_t offset, uint16_t value) +void biosram_write16(uint8_t reg, uint16_t value) { int i; for (i = 0 ; i < sizeof(value) ; i++) { - biosram_write8(offset + i, value & 0xff); + biosram_write8(reg + i, value & 0xff); value >>= 8; } }
-void biosram_write32(uint8_t offset, uint32_t value) +void biosram_write32(uint8_t reg, uint32_t value) { int i; for (i = 0 ; i < sizeof(value) ; i++) { - biosram_write8(offset + i, value & 0xff); + biosram_write8(reg + i, value & 0xff); value >>= 8; } } diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 34e9ce9..c90c0c8 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -25,12 +25,12 @@ void pm_io_write8(uint8_t reg, uint8_t value); void pm_io_write16(uint8_t reg, uint16_t value); void pm_io_write32(uint8_t reg, uint32_t value); -uint8_t smi_read8(uint8_t offset); -uint16_t smi_read16(uint8_t offset); -uint32_t smi_read32(uint8_t offset); -void smi_write8(uint8_t offset, uint8_t value); -void smi_write16(uint8_t offset, uint16_t value); -void smi_write32(uint8_t offset, uint32_t value); +uint8_t smi_read8(uint8_t reg); +uint16_t smi_read16(uint8_t reg); +uint32_t smi_read32(uint8_t reg); +void smi_write8(uint8_t reg, uint8_t value); +void smi_write16(uint8_t reg, uint16_t value); +void smi_write32(uint8_t reg, uint32_t value); uint8_t pm_read8(uint8_t reg); uint16_t pm_read16(uint8_t reg); uint32_t pm_read32(uint8_t reg); @@ -43,12 +43,12 @@ void pm2_write8(uint8_t reg, uint8_t value); void pm2_write16(uint8_t reg, uint16_t value); void pm2_write32(uint8_t reg, uint32_t value); -uint8_t biosram_read8(uint8_t offset); -uint16_t biosram_read16(uint8_t offset); -uint32_t biosram_read32(uint8_t offset); -void biosram_write8(uint8_t offset, uint8_t value); -void biosram_write16(uint8_t offset, uint16_t value); -void biosram_write32(uint8_t offset, uint32_t value); +uint8_t biosram_read8(uint8_t reg); +uint16_t biosram_read16(uint8_t reg); +uint32_t biosram_read32(uint8_t reg); +void biosram_write8(uint8_t reg, uint8_t value); +void biosram_write16(uint8_t reg, uint16_t value); +void biosram_write32(uint8_t reg, uint32_t value); uint8_t acpi_read8(uint8_t reg); uint16_t acpi_read16(uint8_t reg); uint32_t acpi_read32(uint8_t reg);
-
Furquan Shaikh (Code Review) -
Marshall Dawson (Code Review)
-
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)