Change in coreboot[master]: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45891 )

Change subject: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform.
......................................................................

sb/amd/*/*/smbus.c: Make 'smbus.c' uniform.

Reformat 'smbus.c' files and drop unuseful BIOS_DEBUG messages.

Change-Id: I1efddef31dbbb97a7c75c7432d5fe8913a7c3e50
Signed-off-by: Elyes HAOUAS 
---
M src/southbridge/amd/agesa/hudson/smbus.c
M src/southbridge/amd/cimx/sb800/smbus.c
M src/southbridge/amd/pi/hudson/smbus.c
3 files changed, 61 insertions(+), 63 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/45891/1

diff --git a/src/southbridge/amd/agesa/hudson/smbus.c b/src/southbridge/amd/agesa/hudson/smbus.c
index c418492..a8bf85f 100644
--- a/src/southbridge/amd/agesa/hudson/smbus.c
+++ b/src/southbridge/amd/agesa/hudson/smbus.c
@@ -4,12 +4,15 @@
 #define _HUDSON_SMBUS_C_

#include <arch/io.h>
+#include <console/console.h>
 #include <stdint.h>
+
 #include "smbus.h"

static int smbus_wait_until_ready(u32 smbus_io_base)
 {
    u32 loops;
+
    loops = SMBUS_TIMEOUT;
    do {
    	u8 val;
@@ -20,12 +23,14 @@
    	}
    	outb(val, smbus_io_base + SMBHSTSTAT);
    } while (--loops);
+
    return -2;		/* time out */
 }

static int smbus_wait_until_done(u32 smbus_io_base)
 {
    u32 loops;
+
    loops = SMBUS_TIMEOUT;
    do {
    	u8 val;
@@ -40,6 +45,7 @@
    		return 0;
    	}
    } while (--loops);
+
    return -3;		/* timeout */
 }

@@ -48,10 +54,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the device I'm talking to */
+	printk(BIOS_DEBUG, "%s - set the device I'm talking to.\n", __func__);
    outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);

byte = inb(smbus_io_base + SMBHSTCTRL);
@@ -75,10 +82,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the command... */
+	 printk(BIOS_DEBUG, "%s - set the command.\n", __func__);
    outb(val, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -97,16 +105,16 @@
    return 0;
 }

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

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the command/address... */
+	printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__);
    outb(address & 0xff, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -128,16 +136,16 @@
    return byte;
 }

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

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the command/address... */
+	printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__);
    outb(address & 0xff, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -159,8 +167,7 @@
    return 0;
 }

-void alink_ab_indx(u32 reg_space, u32 reg_addr,
-			  u32 mask, u32 val)
+void alink_ab_indx(u32 reg_space, u32 reg_addr, u32 mask, u32 val)
 {
    u32 tmp;

@@ -181,8 +188,7 @@
    outl(0, AB_INDX);
 }

-void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port,
-			  u32 mask, u32 val)
+void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val)
 {
    u32 tmp;

@@ -206,8 +212,7 @@
 /* space = 0: AX_INDXC, AX_DATAC
  * space = 1: AX_INDXP, AX_DATAP
  */
-void alink_ax_indx(u32 space /*c or p? */, u32 axindc,
-			  u32 mask, u32 val)
+void alink_ax_indx(u32 space /*c or p? */, u32 axindc, u32 mask, u32 val)
 {
    u32 tmp;

diff --git a/src/southbridge/amd/cimx/sb800/smbus.c b/src/southbridge/amd/cimx/sb800/smbus.c
index 7d1ffb9..460b96f 100644
--- a/src/southbridge/amd/cimx/sb800/smbus.c
+++ b/src/southbridge/amd/cimx/sb800/smbus.c
@@ -1,8 +1,10 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <arch/io.h>
+#include <console/console.h>
+#include <stdint.h>
+
 #include "smbus.h"
-#include <console/console.h>	/* printk */

static int smbus_wait_until_ready(u32 smbus_io_base)
 {
@@ -49,12 +51,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__);
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
-	/* set the device I'm talking to */
+	printk(BIOS_DEBUG, "%s - set the device I'm talking to.\n", __func__);
    outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);

byte = inb(smbus_io_base + SMBHSTCTRL);
@@ -70,7 +71,6 @@
    /* read results of transaction */
    byte = inb(smbus_io_base + SMBHSTCMD);

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
    return byte;
 }

@@ -79,12 +79,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__);
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	 printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
-	/* set the command... */
+	 printk(BIOS_DEBUG, "%s - set the command.\n", __func__);
    outb(val, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -100,7 +99,6 @@
    	return -3;	/* timeout or error */
    }

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
    return 0;
 }

@@ -109,12 +107,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__);
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
-	/* set the command/address... */
+	printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__);
    outb(address & 0xff, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -133,7 +130,6 @@
    /* read results of transaction */
    byte = inb(smbus_io_base + SMBHSTDAT0);

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
    return byte;
 }

@@ -142,12 +138,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__);
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
-	/* set the command/address... */
+	printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__);
    outb(address & 0xff, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -166,7 +161,6 @@
    	return -3;	/* timeout or error */
    }

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
    return 0;
 }

@@ -174,7 +168,6 @@
 {
    u32 tmp;

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
    outl((reg_space & 0x7) << 29 | reg_addr, AB_INDX);
    tmp = inl(AB_DATA);
    /* rpr 4.2
@@ -190,14 +183,12 @@
    outl((reg_space & 0x7) << 29 | reg_addr, AB_INDX);	/* probably we don't have to do it again. */
    outl(tmp, AB_DATA);
    outl(0, AB_INDX);
-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
 }

void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val)
 {
    u32 tmp;

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
    outl((reg_space & 0x7) << 29 | (port & 3) << 24 | reg_addr, AB_INDX);
    tmp = inl(AB_DATA);
    /* rpr 4.2
@@ -213,7 +204,6 @@
    outl((reg_space & 0x7) << 29 | (port & 3) << 24 | reg_addr, AB_INDX);	/* probably we don't have to do it again. */
    outl(tmp, AB_DATA);
    outl(0, AB_INDX);
-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
 }

/* space = 0: AX_INDXC, AX_DATAC
@@ -223,7 +213,6 @@
 {
    u32 tmp;

-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__);
    /* read axindc to tmp */
    outl(space << 29 | space << 3 | 0x30, AB_INDX);
    outl(axindc, AB_DATA);
@@ -242,5 +231,4 @@
    outl(space << 29 | space << 3 | 0x34, AB_INDX);
    outl(tmp, AB_DATA);
    outl(0, AB_INDX);
-	printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__);
 }
diff --git a/src/southbridge/amd/pi/hudson/smbus.c b/src/southbridge/amd/pi/hudson/smbus.c
index c418492..a8bf85f 100644
--- a/src/southbridge/amd/pi/hudson/smbus.c
+++ b/src/southbridge/amd/pi/hudson/smbus.c
@@ -4,12 +4,15 @@
 #define _HUDSON_SMBUS_C_

#include <arch/io.h>
+#include <console/console.h>
 #include <stdint.h>
+
 #include "smbus.h"

static int smbus_wait_until_ready(u32 smbus_io_base)
 {
    u32 loops;
+
    loops = SMBUS_TIMEOUT;
    do {
    	u8 val;
@@ -20,12 +23,14 @@
    	}
    	outb(val, smbus_io_base + SMBHSTSTAT);
    } while (--loops);
+
    return -2;		/* time out */
 }

static int smbus_wait_until_done(u32 smbus_io_base)
 {
    u32 loops;
+
    loops = SMBUS_TIMEOUT;
    do {
    	u8 val;
@@ -40,6 +45,7 @@
    		return 0;
    	}
    } while (--loops);
+
    return -3;		/* timeout */
 }

@@ -48,10 +54,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the device I'm talking to */
+	printk(BIOS_DEBUG, "%s - set the device I'm talking to.\n", __func__);
    outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);

byte = inb(smbus_io_base + SMBHSTCTRL);
@@ -75,10 +82,11 @@
    u8 byte;

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the command... */
+	 printk(BIOS_DEBUG, "%s - set the command.\n", __func__);
    outb(val, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -97,16 +105,16 @@
    return 0;
 }

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

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the command/address... */
+	printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__);
    outb(address & 0xff, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -128,16 +136,16 @@
    return byte;
 }

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

if (smbus_wait_until_ready(smbus_io_base) < 0) {
-		return -2;	/* not ready */
+		printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__);
+		return -2;
    }

-	/* set the command/address... */
+	printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__);
    outb(address & 0xff, smbus_io_base + SMBHSTCMD);

/* set the device I'm talking to */
@@ -159,8 +167,7 @@
    return 0;
 }

-void alink_ab_indx(u32 reg_space, u32 reg_addr,
-			  u32 mask, u32 val)
+void alink_ab_indx(u32 reg_space, u32 reg_addr, u32 mask, u32 val)
 {
    u32 tmp;

@@ -181,8 +188,7 @@
    outl(0, AB_INDX);
 }

-void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port,
-			  u32 mask, u32 val)
+void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val)
 {
    u32 tmp;

@@ -206,8 +212,7 @@
 /* space = 0: AX_INDXC, AX_DATAC
  * space = 1: AX_INDXP, AX_DATAP
  */
-void alink_ax_indx(u32 space /*c or p? */, u32 axindc,
-			  u32 mask, u32 val)
+void alink_ax_indx(u32 space /*c or p? */, u32 axindc, u32 mask, u32 val)
 {
    u32 tmp;

[Angel Pons (Code Review)]

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

Change subject: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform.
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/45891/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45891/2//COMMIT_MSG@7 
PS2, Line 7: .
no trailing period in the commit summary

https://review.coreboot.org/c/coreboot/+/45891/2//COMMIT_MSG@9 
PS2, Line 9: drop unuseful BIOS_DEBUG messages
Quite the opposite, I'd say. This change adds useless BIOS_DEBUG messages on Hudson.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45891 )

Change subject: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform.
......................................................................

Patch Set 2:

(2 comments)

Thank you.

https://review.coreboot.org/c/coreboot/+/45891/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45891/2//COMMIT_MSG@7 
PS2, Line 7: .
> no trailing period in the commit summary

Done

https://review.coreboot.org/c/coreboot/+/45891/2//COMMIT_MSG@9 
PS2, Line 9: drop unuseful BIOS_DEBUG messages
> Quite the opposite, I'd say. This change adds useless BIOS_DEBUG messages on Hudson.

Done

Change in coreboot[master]: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform
......................................................................

sb/amd/*/*/smbus.c: Make 'smbus.c' uniform

Reformat 'smbus.c' files and drop useless BIOS_DEBUG messages.

Change-Id: I1efddef31dbbb97a7c75c7432d5fe8913a7c3e50
Signed-off-by: Elyes HAOUAS 
---
M src/southbridge/amd/agesa/hudson/smbus.c
M src/southbridge/amd/cimx/sb800/smbus.c
M src/southbridge/amd/pi/hudson/smbus.c
3 files changed, 49 insertions(+), 69 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/45891/3