Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/28565 )
Change subject: util/board_status: Add support of CMOS values dump
......................................................................
util/board_status: Add support of CMOS values dump
Change-Id: I89f9a0e9622557b01dda52378f8f1323777bce39
Signed-off-by: Evgeny Zinoviev <me(a)ch1p.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/28565
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net>
Reviewed-by: Peter Lemenkov <lemenkov(a)gmail.com>
Reviewed-by: Patrick Georgi <pgeorgi(a)google.com>
---
M util/board_status/board_status.sh
1 file changed, 47 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Patrick Georgi: Looks good to me, approved
Paul Menzel: Looks good to me, but someone else must approve
Peter Lemenkov: Looks good to me, but someone else must approve
diff --git a/util/board_status/board_status.sh b/util/board_status/board_status.sh
index 0dc96e8..6d6854d 100755
--- a/util/board_status/board_status.sh
+++ b/util/board_status/board_status.sh
@@ -27,6 +27,9 @@
# Used if cbmem is not in default $PATH, e.g. not installed or when using `sudo`
CBMEM_PATH=""
+# Used if nvramtool is not in default $PATH, e.g. not installed or when using `sudo`
+NVRAMTOOL_PATH=""
+
# test a command
#
# $1: 0 ($LOCAL) to run command locally,
@@ -176,6 +179,8 @@
Options
-c, --cbmem
Path to cbmem on device under test (DUT).
+ -n, --nvramtool
+ Path to nvramtool on device under test (DUT).
-C, --clobber
Clobber temporary output when finished. Useful for debugging.
-h, --help
@@ -207,7 +212,7 @@
LONGOPTS="${LONGOPTS},serial-device:,serial-speed:"
LONGOPTS="${LONGOPTS},ssh-port:"
-ARGS=$(getopt -o c:Chi:r:s:S:u -l "$LONGOPTS" -n "$0" -- "$@");
+ARGS=$(getopt -o c:n:Chi:r:s:S:u -l "$LONGOPTS" -n "$0" -- "$@");
if [ $? != 0 ] ; then echo "Terminating..." >&2 ; exit 1 ; fi
eval set -- "$ARGS"
while true ; do
@@ -217,6 +222,10 @@
shift
CBMEM_PATH="$1"
;;
+ -n|--nvramtool)
+ shift
+ NVRAMTOOL_PATH="$1"
+ ;;
-C|--clobber)
CLOBBER_OUTPUT=1
;;
@@ -370,6 +379,17 @@
cbmem_cmd="cbmem"
fi
+cmos_enabled=0
+if grep -q "CONFIG_USE_OPTION_TABLE=y" "${tmpdir}/${results}/config.short.txt" > /dev/null; then
+ cmos_enabled=1
+fi
+
+if [ -n "$NVRAMTOOL_PATH" ]; then
+ nvramtool_cmd="$NVRAMTOOL_PATH"
+else
+ nvramtool_cmd="nvramtool"
+fi
+
if [ -n "$SERIAL_DEVICE" ]; then
get_serial_bootlog "$SERIAL_DEVICE" "$SERIAL_PORT_SPEED" "${tmpdir}/${results}/coreboot_console.txt"
elif [ -n "$REMOTE_HOST" ]; then
@@ -380,6 +400,13 @@
echo "Getting timestamp data"
cmd_nonfatal $REMOTE "$cbmem_cmd -t" "${tmpdir}/${results}/coreboot_timestamps.txt"
+ if [ "$cmos_enabled" -eq 1 ]; then
+ echo "Verifying that nvramtool is available on remote device"
+ test_cmd $REMOTE "$nvramtool_cmd"
+ echo "Getting all CMOS values"
+ cmd $REMOTE "$nvramtool_cmd -a" "${tmpdir}/${results}/cmos_values.txt"
+ fi
+
echo "Getting remote dmesg"
cmd $REMOTE dmesg "${tmpdir}/${results}/kernel_log.txt"
else
@@ -402,6 +429,25 @@
echo "Getting timestamp data"
cmd_nonfatal $LOCAL "$cbmem_cmd -t" "${tmpdir}/${results}/coreboot_timestamps.txt"
+ if [ "$cmos_enabled" -eq 1 ]; then
+ echo "Verifying that nvramtool is available"
+ if [ $(id -u) -ne 0 ]; then
+ command -v "$nvramtool_cmd" >/dev/null
+ if [ $? -ne 0 ]; then
+ echo "Failed to run $nvramtool_cmd. Check \$PATH or" \
+ "use -n to specify path to nvramtool binary."
+ exit $EXIT_FAILURE
+ else
+ nvramtool_cmd="sudo $nvramtool_cmd"
+ fi
+ else
+ test_cmd $LOCAL "$nvramtool_cmd"
+ fi
+
+ echo "Getting all CMOS values"
+ cmd $LOCAL "$nvramtool_cmd -a" "${tmpdir}/${results}/cmos_values.txt"
+ fi
+
echo "Getting local dmesg"
cmd $LOCAL "sudo dmesg" "${tmpdir}/${results}/kernel_log.txt"
fi
--
To view, visit https://review.coreboot.org/c/coreboot/+/28565
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I89f9a0e9622557b01dda52378f8f1323777bce39
Gerrit-Change-Number: 28565
Gerrit-PatchSet: 3
Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Peter Lemenkov <lemenkov(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: merged
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28565 )
Change subject: util/board_status: Add support of CMOS values dump
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/28565/2/util/board_status/board_st…
File util/board_status/board_status.sh:
https://review.coreboot.org/c/coreboot/+/28565/2/util/board_status/board_st…
PS2, Line 387: if [ -n "$NVRAMTOOL_PATH" ]; then
> instead of this extensive test, why not just initialize nvramtool_cmd to "nvramtool" and override th […]
This is potentially for a follow-up.
--
To view, visit https://review.coreboot.org/c/coreboot/+/28565
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I89f9a0e9622557b01dda52378f8f1323777bce39
Gerrit-Change-Number: 28565
Gerrit-PatchSet: 2
Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Peter Lemenkov <lemenkov(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Tue, 10 Mar 2020 20:20:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: comment
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28565 )
Change subject: util/board_status: Add support of CMOS values dump
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
> Patch Set 2:
> It doesn't hurt in the report... but this script is too powerful already
> for my taste (and should have had a clean redesign years ago, imho).
It should, yes. But that's outside the scope of this change ;-)
https://review.coreboot.org/c/coreboot/+/28565/2/util/board_status/board_st…
File util/board_status/board_status.sh:
https://review.coreboot.org/c/coreboot/+/28565/2/util/board_status/board_st…
PS2, Line 387: if [ -n "$NVRAMTOOL_PATH" ]; then
instead of this extensive test, why not just initialize nvramtool_cmd to "nvramtool" and override that in the -n handler?
--
To view, visit https://review.coreboot.org/c/coreboot/+/28565
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I89f9a0e9622557b01dda52378f8f1323777bce39
Gerrit-Change-Number: 28565
Gerrit-PatchSet: 2
Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Peter Lemenkov <lemenkov(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Tue, 10 Mar 2020 20:19:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37871 )
Change subject: soc/intel/common/block: Enable PMC IPC driver
......................................................................
Patch Set 20:
We don't need to abandon this series. This is still going to be required for enabling external display support.
--
To view, visit https://review.coreboot.org/c/coreboot/+/37871
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd3ed262fc700ccc891ec68997a108f5bfbaf9ed
Gerrit-Change-Number: 37871
Gerrit-PatchSet: 20
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Divya Sasidharan <divya.s.sasidharan(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Tue, 10 Mar 2020 17:29:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37871 )
Change subject: soc/intel/common/block: Enable PMC IPC driver
......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37871/20/src/soc/intel/common/bloc…
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/37871/20/src/soc/intel/common/bloc…
PS20, Line 655: rbuf->buf0 = read32((void *)(pmcbase + PMC_IPC_RBUF0));
> should that check for rbuf == NULL in case you don't have a response?
if the pmc ipc command is successful the entire buffer should be replaced with relevant return information. so the check for error prior to this should be enough to handle this
--
To view, visit https://review.coreboot.org/c/coreboot/+/37871
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd3ed262fc700ccc891ec68997a108f5bfbaf9ed
Gerrit-Change-Number: 37871
Gerrit-PatchSet: 20
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Divya Sasidharan <divya.s.sasidharan(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Tue, 10 Mar 2020 16:51:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: comment
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37871 )
Change subject: soc/intel/common/block: Enable PMC IPC driver
......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37871/20/src/soc/intel/common/bloc…
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/37871/20/src/soc/intel/common/bloc…
PS20, Line 655: rbuf->buf0 = read32((void *)(pmcbase + PMC_IPC_RBUF0));
should that check for rbuf == NULL in case you don't have a response?
--
To view, visit https://review.coreboot.org/c/coreboot/+/37871
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd3ed262fc700ccc891ec68997a108f5bfbaf9ed
Gerrit-Change-Number: 37871
Gerrit-PatchSet: 20
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Divya Sasidharan <divya.s.sasidharan(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Tue, 10 Mar 2020 14:32:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment