Change in coreboot[master]: soc/amd/picasso: move to using smbus_host.h definitions

[Aaron Durbin (Code Review)]

Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38615 )

Change subject: soc/amd/picasso: move to using smbus_host.h definitions
......................................................................

soc/amd/picasso: move to using smbus_host.h definitions

The SMBus function declarations were duplicated. Use the common
ones provided by smbus_host.h.

Change-Id: Ia8fec8f58d72690d73f2241e69b3ff05f74943a4
Signed-off-by: Aaron Durbin 
---
D src/soc/amd/picasso/include/soc/smbus.h
M src/soc/amd/picasso/sm.c
M src/soc/amd/picasso/smbus.c
M src/soc/amd/picasso/southbridge.c
4 files changed, 12 insertions(+), 40 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38615/1

diff --git a/src/soc/amd/picasso/include/soc/smbus.h b/src/soc/amd/picasso/include/soc/smbus.h
deleted file mode 100644
index 524be77..0000000
--- a/src/soc/amd/picasso/include/soc/smbus.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright (C) 2010 Advanced Micro Devices, Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __PICASSO_SMBUS_H__
-#define __PICASSO_SMBUS_H__
-
-#include <stdint.h>
-#include <soc/iomap.h>
-
-int do_smbus_read_byte(u32 mmio, u8 device, u8 address);
-int do_smbus_write_byte(u32 mmio, u8 device, u8 address, u8 val);
-int do_smbus_recv_byte(u32 mmio, u8 device);
-int do_smbus_send_byte(u32 mmio, u8 device, u8 val);
-
-#endif /* __PICASSO_SMBUS_H__ */
diff --git a/src/soc/amd/picasso/sm.c b/src/soc/amd/picasso/sm.c
index 438909d..f0ba559 100644
--- a/src/soc/amd/picasso/sm.c
+++ b/src/soc/amd/picasso/sm.c
@@ -18,10 +18,10 @@
 #include <device/pci_ids.h>
 #include <device/pci_ops.h>
 #include <device/smbus.h>
+#include <device/smbus_host.h>
 #include <cpu/x86/lapic.h>
 #include <arch/ioapic.h>
 #include <soc/southbridge.h>
-#include <soc/smbus.h>

/*
 * The southbridge enables all USB controllers by default in SMBUS Control.
diff --git a/src/soc/amd/picasso/smbus.c b/src/soc/amd/picasso/smbus.c
index f5a9d60..5474c5c 100644
--- a/src/soc/amd/picasso/smbus.c
+++ b/src/soc/amd/picasso/smbus.c
@@ -15,8 +15,8 @@

#include <stdint.h>
 #include <console/console.h>
+#include <device/smbus_host.h>
 #include <amdblocks/acpimmio.h>
-#include <soc/smbus.h>
 #include <soc/southbridge.h>

/*
@@ -25,7 +25,7 @@
  */
 #define SMBUS_TIMEOUT (100 * 1000 * 10)

-static u8 controller_read8(u32 base, u8 reg)
+static u8 controller_read8(uintptr_t base, u8 reg)
 {
    switch (base) {
    case ACPIMMIO_SMBUS_BASE:
@@ -33,13 +33,13 @@
    case ACPIMMIO_ASF_BASE:
    	return asf_read8(reg);
    default:
-		printk(BIOS_ERR, "Error attempting to read SMBus at address 0x%x\n",
+		printk(BIOS_ERR, "Error attempting to read SMBus at address 0x%lx\n",
    			base);
    }
    return 0xff;
 }

-static void controller_write8(u32 base, u8 reg, u8 val)
+static void controller_write8(uintptr_t base, u8 reg, u8 val)
 {
    switch (base) {
    case ACPIMMIO_SMBUS_BASE:
@@ -49,12 +49,12 @@
    	asf_write8(reg, val);
    	break;
    default:
-		printk(BIOS_ERR, "Error attempting to write SMBus at address 0x%x\n",
+		printk(BIOS_ERR, "Error attempting to write SMBus at address 0x%lx\n",
    			base);
    }
 }

-static int smbus_wait_until_ready(u32 mmio)
+static int smbus_wait_until_ready(uintptr_t mmio)
 {
    u32 loops;
    loops = SMBUS_TIMEOUT;
@@ -70,7 +70,7 @@
    return -2;		/* time out */
 }

-static int smbus_wait_until_done(u32 mmio)
+static int smbus_wait_until_done(uintptr_t mmio)
 {
    u32 loops;
    loops = SMBUS_TIMEOUT;
@@ -89,7 +89,7 @@
    return -3;		/* timeout */
 }

-int do_smbus_recv_byte(u32 mmio, u8 device)
+int do_smbus_recv_byte(uintptr_t mmio, u8 device)
 {
    u8 byte;

@@ -114,7 +114,7 @@
    return byte;
 }

-int do_smbus_send_byte(u32 mmio, u8 device, u8 val)
+int do_smbus_send_byte(uintptr_t mmio, u8 device, u8 val)
 {
    u8 byte;

@@ -139,7 +139,7 @@
    return 0;
 }

-int do_smbus_read_byte(u32 mmio, u8 device, u8 address)
+int do_smbus_read_byte(uintptr_t mmio, u8 device, u8 address)
 {
    u8 byte;

@@ -167,7 +167,7 @@
    return byte;
 }

-int do_smbus_write_byte(u32 mmio, u8 device, u8 address, u8 val)
+int do_smbus_write_byte(uintptr_t mmio, u8 device, u8 address, u8 val)
 {
    u8 byte;

diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c
index 310d9a2..56fe88d 100644
--- a/src/soc/amd/picasso/southbridge.c
+++ b/src/soc/amd/picasso/southbridge.c
@@ -29,7 +29,6 @@
 #include <amdblocks/acpi.h>
 #include <soc/cpu.h>
 #include <soc/southbridge.h>
-#include <soc/smbus.h>
 #include <soc/smi.h>
 #include <soc/amd_pci_int_defs.h>
 #include <delay.h>

[Aaron Durbin (Code Review)]

Aaron Durbin has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/38615 )

Change subject: soc/amd/picasso: move to using smbus_host.h definitions
......................................................................

soc/amd/picasso: move to using smbus_host.h definitions

The SMBus function declarations were duplicated. Use the common
ones provided by smbus_host.h.

Change-Id: Ia8fec8f58d72690d73f2241e69b3ff05f74943a4
Signed-off-by: Aaron Durbin 
---
D src/soc/amd/picasso/include/soc/smbus.h
M src/soc/amd/picasso/sm.c
M src/soc/amd/picasso/smbus.c
M src/soc/amd/picasso/southbridge.c
4 files changed, 12 insertions(+), 40 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38615/2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38615 )

Change subject: soc/amd/picasso: move to using smbus_host.h definitions
......................................................................

Patch Set 2: Code-Review+1

[Angel Pons (Code Review)]

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

Change subject: soc/amd/picasso: move to using smbus_host.h definitions
......................................................................

Patch Set 3: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38615 )

Change subject: soc/amd/picasso: move to using smbus_host.h definitions
......................................................................

soc/amd/picasso: move to using smbus_host.h definitions

The SMBus function declarations were duplicated. Use the common
ones provided by smbus_host.h.

Change-Id: Ia8fec8f58d72690d73f2241e69b3ff05f74943a4
Signed-off-by: Aaron Durbin 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38615
Reviewed-by: Angel Pons 
Reviewed-by: Kyösti Mälkki 
Tested-by: build bot (Jenkins) 
---
D src/soc/amd/picasso/include/soc/smbus.h
M src/soc/amd/picasso/sm.c
M src/soc/amd/picasso/smbus.c
M src/soc/amd/picasso/southbridge.c
4 files changed, 12 insertions(+), 40 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/amd/picasso/include/soc/smbus.h b/src/soc/amd/picasso/include/soc/smbus.h
deleted file mode 100644
index 524be77..0000000
--- a/src/soc/amd/picasso/include/soc/smbus.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright (C) 2010 Advanced Micro Devices, Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __PICASSO_SMBUS_H__
-#define __PICASSO_SMBUS_H__
-
-#include <stdint.h>
-#include <soc/iomap.h>
-
-int do_smbus_read_byte(u32 mmio, u8 device, u8 address);
-int do_smbus_write_byte(u32 mmio, u8 device, u8 address, u8 val);
-int do_smbus_recv_byte(u32 mmio, u8 device);
-int do_smbus_send_byte(u32 mmio, u8 device, u8 val);
-
-#endif /* __PICASSO_SMBUS_H__ */
diff --git a/src/soc/amd/picasso/sm.c b/src/soc/amd/picasso/sm.c
index 438909d..f0ba559 100644
--- a/src/soc/amd/picasso/sm.c
+++ b/src/soc/amd/picasso/sm.c
@@ -18,10 +18,10 @@
 #include <device/pci_ids.h>
 #include <device/pci_ops.h>
 #include <device/smbus.h>
+#include <device/smbus_host.h>
 #include <cpu/x86/lapic.h>
 #include <arch/ioapic.h>
 #include <soc/southbridge.h>
-#include <soc/smbus.h>

/*
 * The southbridge enables all USB controllers by default in SMBUS Control.
diff --git a/src/soc/amd/picasso/smbus.c b/src/soc/amd/picasso/smbus.c
index f5a9d60..5474c5c 100644
--- a/src/soc/amd/picasso/smbus.c
+++ b/src/soc/amd/picasso/smbus.c
@@ -15,8 +15,8 @@

#include <stdint.h>
 #include <console/console.h>
+#include <device/smbus_host.h>
 #include <amdblocks/acpimmio.h>
-#include <soc/smbus.h>
 #include <soc/southbridge.h>

/*
@@ -25,7 +25,7 @@
  */
 #define SMBUS_TIMEOUT (100 * 1000 * 10)

-static u8 controller_read8(u32 base, u8 reg)
+static u8 controller_read8(uintptr_t base, u8 reg)
 {
    switch (base) {
    case ACPIMMIO_SMBUS_BASE:
@@ -33,13 +33,13 @@
    case ACPIMMIO_ASF_BASE:
    	return asf_read8(reg);
    default:
-		printk(BIOS_ERR, "Error attempting to read SMBus at address 0x%x\n",
+		printk(BIOS_ERR, "Error attempting to read SMBus at address 0x%lx\n",
    			base);
    }
    return 0xff;
 }

-static void controller_write8(u32 base, u8 reg, u8 val)
+static void controller_write8(uintptr_t base, u8 reg, u8 val)
 {
    switch (base) {
    case ACPIMMIO_SMBUS_BASE:
@@ -49,12 +49,12 @@
    	asf_write8(reg, val);
    	break;
    default:
-		printk(BIOS_ERR, "Error attempting to write SMBus at address 0x%x\n",
+		printk(BIOS_ERR, "Error attempting to write SMBus at address 0x%lx\n",
    			base);
    }
 }

-static int smbus_wait_until_ready(u32 mmio)
+static int smbus_wait_until_ready(uintptr_t mmio)
 {
    u32 loops;
    loops = SMBUS_TIMEOUT;
@@ -70,7 +70,7 @@
    return -2;		/* time out */
 }

-static int smbus_wait_until_done(u32 mmio)
+static int smbus_wait_until_done(uintptr_t mmio)
 {
    u32 loops;
    loops = SMBUS_TIMEOUT;
@@ -89,7 +89,7 @@
    return -3;		/* timeout */
 }

-int do_smbus_recv_byte(u32 mmio, u8 device)
+int do_smbus_recv_byte(uintptr_t mmio, u8 device)
 {
    u8 byte;

@@ -114,7 +114,7 @@
    return byte;
 }

-int do_smbus_send_byte(u32 mmio, u8 device, u8 val)
+int do_smbus_send_byte(uintptr_t mmio, u8 device, u8 val)
 {
    u8 byte;

@@ -139,7 +139,7 @@
    return 0;
 }

-int do_smbus_read_byte(u32 mmio, u8 device, u8 address)
+int do_smbus_read_byte(uintptr_t mmio, u8 device, u8 address)
 {
    u8 byte;

@@ -167,7 +167,7 @@
    return byte;
 }

-int do_smbus_write_byte(u32 mmio, u8 device, u8 address, u8 val)
+int do_smbus_write_byte(uintptr_t mmio, u8 device, u8 address, u8 val)
 {
    u8 byte;

diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c
index 310d9a2..56fe88d 100644
--- a/src/soc/amd/picasso/southbridge.c
+++ b/src/soc/amd/picasso/southbridge.c
@@ -29,7 +29,6 @@
 #include <amdblocks/acpi.h>
 #include <soc/cpu.h>
 #include <soc/southbridge.h>
-#include <soc/smbus.h>
 #include <soc/smi.h>
 #include <soc/amd_pci_int_defs.h>
 #include <delay.h>

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38615 )

Change subject: soc/amd/picasso: move to using smbus_host.h definitions
......................................................................

Patch Set 4:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/365
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/364
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/363

Please note: This test is under development and might not be accurate at all!