Change in coreboot[master]: libpayload/drivers/i8042: add error message of i8042_probe

[EricR Lai (Code Review)]

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error message of i8042_probe
......................................................................

libpayload/drivers/i8042: add error message of i8042_probe

Print error message before error return for better debugging.

Signed-off-by: Eric Lai 
Change-Id: I52039dcab72c6295dfb6b887a7000a6d2bd050ee
---
M payloads/libpayload/drivers/i8042/i8042.c
1 file changed, 9 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37689/1

diff --git a/payloads/libpayload/drivers/i8042/i8042.c b/payloads/libpayload/drivers/i8042/i8042.c
index e97fab8..b9bafa7 100644
--- a/payloads/libpayload/drivers/i8042/i8042.c
+++ b/payloads/libpayload/drivers/i8042/i8042.c
@@ -197,23 +197,29 @@

/* If 0x64 returns 0xff, then we have no keyboard
     * controller */
-	if (read_status() == 0xFF)
+	if (read_status() == 0xFF) {
+		printf("%s:No keyboard controller exist!\n", __func__);
    	return 0;
+	}

-	if (!i8042_wait_cmd_rdy())
+	if (!i8042_wait_cmd_rdy()) {
+		printf("ERROR: i8042_wait_cmd_rdy failed!\n");
    	return 0;
+	}

kbc_init = 1;

/* Disable first device */
    if (i8042_cmd(I8042_CMD_DIS_KB) != 0) {
    	kbc_init = 0;
+		printf("ERROR: i8042_cmd I8042_CMD_DIS_KB failed!\n");
    	return 0;
    }

/* Disable second device */
    if (i8042_cmd(I8042_CMD_DIS_AUX) != 0) {
    	kbc_init = 0;
+		printf("ERROR: i8042_cmd I8042_CMD_DIS_AUX failed!\n");
    	return 0;
    }

@@ -225,6 +231,7 @@
    if (i8042_cmd_with_response(I8042_CMD_SELF_TEST)
        != I8042_SELF_TEST_RSP) {
    	kbc_init = 0;
+		printf("ERROR: i8042_cmd I8042_CMD_SELF_TEST failed!\n");
    	return 0;
    }

[Furquan Shaikh (Code Review)]

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

Change subject: libpayload/drivers/i8042: add error message of i8042_probe
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
File payloads/libpayload/drivers/i8042/i8042.c:

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
PS1, Line 201: __func__
Why is __func__ added to this print, but not the rest?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error message of i8042_probe
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
File payloads/libpayload/drivers/i8042/i8042.c:

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
PS1, Line 201: __func__
> Why is __func__ added to this print, but not the rest?

For the 80 char lol. And i8042 should be okay like the log in keyboard.c?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error message of i8042_probe
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
File payloads/libpayload/drivers/i8042/i8042.c:

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
PS1, Line 201: __func__
> For the 80 char lol. And i8042 should be okay like the log in keyboard. […]

Oh, I think you mean put just ERROR here? If so, I can change it.

[EricR Lai (Code Review)]

Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: libpayload/drivers/i8042: add error message of i8042_probe
......................................................................

libpayload/drivers/i8042: add error message of i8042_probe

Print error message before error return for better debugging.

Signed-off-by: Eric Lai 
Change-Id: I52039dcab72c6295dfb6b887a7000a6d2bd050ee
---
M payloads/libpayload/drivers/i8042/i8042.c
1 file changed, 9 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37689/2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error message of i8042_probe
......................................................................

Patch Set 2:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/37689/2//COMMIT_MSG@7 
PS2, Line 7: libpayload/drivers/i8042: add error message of i8042_probe
> add error messages to i8042_probe()

https://review.coreboot.org/c/coreboot/+/37689/2/payloads/libpayload/drivers... 
File payloads/libpayload/drivers/i8042/i8042.c:

https://review.coreboot.org/c/coreboot/+/37689/2/payloads/libpayload/drivers... 
PS2, Line 201: exist
exist*s*

Maybe:
> No keyboard controller found

Change in coreboot[master]: libpayload/drivers/i8042: add error messages to i8042_probe

[EricR Lai (Code Review)]

Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: libpayload/drivers/i8042: add error messages to i8042_probe
......................................................................

libpayload/drivers/i8042: add error messages to i8042_probe

Print error message before error return for better debugging.

Signed-off-by: Eric Lai 
Change-Id: I52039dcab72c6295dfb6b887a7000a6d2bd050ee
---
M payloads/libpayload/drivers/i8042/i8042.c
1 file changed, 9 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37689/3

Change in coreboot[master]: libpayload/drivers/i8042: add error messages to i8042_probe

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error messages to i8042_probe
......................................................................

Patch Set 3:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/37689/2//COMMIT_MSG@7 
PS2, Line 7: libpayload/drivers/i8042: add error message of i8042_probe
> > add error messages to i8042_probe()

Done

https://review.coreboot.org/c/coreboot/+/37689/2/payloads/libpayload/drivers... 
File payloads/libpayload/drivers/i8042/i8042.c:

https://review.coreboot.org/c/coreboot/+/37689/2/payloads/libpayload/drivers... 
PS2, Line 201: exist
> exist*s* […]

Done

Change in coreboot[master]: libpayload/drivers/i8042: add error messages to i8042_probe

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error messages to i8042_probe
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: libpayload/drivers/i8042: add error messages to i8042_probe

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error messages to i8042_probe
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
File payloads/libpayload/drivers/i8042/i8042.c:

https://review.coreboot.org/c/coreboot/+/37689/1/payloads/libpayload/drivers... 
PS1, Line 201: __func__
> Oh, I think you mean put just ERROR here? If so, I can change it.

Done

Change in coreboot[master]: libpayload/drivers/i8042: add error messages to i8042_probe

[Mathew King (Code Review)]

Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error messages to i8042_probe
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: libpayload/drivers/i8042: add error messages to i8042_probe

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37689 )

Change subject: libpayload/drivers/i8042: add error messages to i8042_probe
......................................................................

libpayload/drivers/i8042: add error messages to i8042_probe

Print error message before error return for better debugging.

Signed-off-by: Eric Lai 
Change-Id: I52039dcab72c6295dfb6b887a7000a6d2bd050ee
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37689
Tested-by: build bot (Jenkins) 
Reviewed-by: Paul Menzel 
Reviewed-by: Mathew King 
---
M payloads/libpayload/drivers/i8042/i8042.c
1 file changed, 9 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Mathew King: Looks good to me, approved

diff --git a/payloads/libpayload/drivers/i8042/i8042.c b/payloads/libpayload/drivers/i8042/i8042.c
index e97fab8..ee9f5fd 100644
--- a/payloads/libpayload/drivers/i8042/i8042.c
+++ b/payloads/libpayload/drivers/i8042/i8042.c
@@ -197,23 +197,29 @@

/* If 0x64 returns 0xff, then we have no keyboard
     * controller */
-	if (read_status() == 0xFF)
+	if (read_status() == 0xFF) {
+		printf("ERROR: No keyboard controller found!\n");
    	return 0;
+	}

-	if (!i8042_wait_cmd_rdy())
+	if (!i8042_wait_cmd_rdy()) {
+		printf("ERROR: i8042_wait_cmd_rdy failed!\n");
    	return 0;
+	}

kbc_init = 1;

/* Disable first device */
    if (i8042_cmd(I8042_CMD_DIS_KB) != 0) {
    	kbc_init = 0;
+		printf("ERROR: i8042_cmd I8042_CMD_DIS_KB failed!\n");
    	return 0;
    }

/* Disable second device */
    if (i8042_cmd(I8042_CMD_DIS_AUX) != 0) {
    	kbc_init = 0;
+		printf("ERROR: i8042_cmd I8042_CMD_DIS_AUX failed!\n");
    	return 0;
    }

@@ -225,6 +231,7 @@
    if (i8042_cmd_with_response(I8042_CMD_SELF_TEST)
        != I8042_SELF_TEST_RSP) {
    	kbc_init = 0;
+		printf("ERROR: i8042_cmd I8042_CMD_SELF_TEST failed!\n");
    	return 0;
    }