Ravi kumar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
trogdor: report hardware watchdog reset after reboot
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 --- M src/soc/qualcomm/sc7180/soc.c 1 file changed, 13 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/1
diff --git a/src/soc/qualcomm/sc7180/soc.c b/src/soc/qualcomm/sc7180/soc.c index 74f0868..b529bf6 100644 --- a/src/soc/qualcomm/sc7180/soc.c +++ b/src/soc/qualcomm/sc7180/soc.c @@ -6,6 +6,11 @@ #include <soc/mmu_common.h> #include <soc/symbols.h> #include <soc/aop.h> +#include <elog.h> +#include <console/console.h> + +#define AOSS_CC_RESET_STATUS 0x0C2F0020 +#define WDOG_RESET_BIT_MASK 0x1
static void soc_read_resources(struct device *dev) { @@ -19,6 +24,14 @@
static void soc_init(struct device *dev) { + volatile unsigned int aoss = *(unsigned int *)AOSS_CC_RESET_STATUS; + + printk(BIOS_INFO, "\nSOC: AOSS_CC_RESET_STATUS : %x\n", aoss & WDOG_RESET_BIT_MASK); + printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss); + + if(aoss & WDOG_RESET_BIT_MASK) + elog_add_event(ELOG_TYPE_ASYNC_HW_TIMER_EXPIRED); + aop_fw_load_reset(); }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 27: volatile unsigned int aoss = *(unsigned int *)AOSS_CC_RESET_STATUS; Guys, how many drivers have we landed by now? You *know* this is not how to do register accesses in coreboot! Please stop making me ask for the same things over and over again in every new patch. :/
There is already a struct sc7180_aoss in <soc/clock.h>, please add this register there and then put a function in the clock driver to access it, e.g. bool clock_last_reset_was_watchdog(void) or something like that. Then call that function from here to decide whether to log the event.
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 30: printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss); This doesn't work for me -- even if the reboot was a watchdog, the register seems to contain a 0. Does it work for you?
To test, I do:
stop daisydog cat > /dev/watchdog
then I press Ctrl+D and wait for the timeout.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 1:
(1 comment)
You should probably reorder this patch towards the back of the train so that we can start landing other patches while this is still being discussed.
https://review.coreboot.org/c/coreboot/+/44868/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44868/1//COMMIT_MSG@8 PS1, Line 8: This is missing a commit message body and a Signed-off-by: line, btw. Patches without that cannot be merged.
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 1:
(2 comments)
Patch Set 1:
(1 comment)
You should probably reorder this patch towards the back of the train so that we can start landing other patches while this is still being discussed.
This patch will be moved to back of train until ready with +2
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 27: volatile unsigned int aoss = *(unsigned int *)AOSS_CC_RESET_STATUS;
Guys, how many drivers have we landed by now? You *know* this is not how to do register accesses in […]
This was put in place by Ravi and T.mike who are not driver developers as a quick test. Ravi will follow the correct procedure
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 30: printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss);
This doesn't work for me -- even if the reboot was a watchdog, the register seems to contain a 0. […]
Kernel needs to be up to date and built without KGDB option, and we have seen this working.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/2/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/2/src/soc/qualcomm/sc7180/soc... PS2, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 30: printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss);
Kernel needs to be up to date and built without KGDB option, and we have seen this working.
Hi julius, Tested these changes, below log for the updating value, when i do reboot from kernel prompt:
SOC: AOSS_CC_RESET_STATUS : 0 AOSS_CC_RESET_STATUS[0x0c2f0020]..[200] FMAP: area FW_MAIN_A found @ 413000 (1302272 bytes)
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 30: printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss);
Hi julius, […]
Comment above added in error, 0x200 refers to PS_HOLD_STATUS not WDOG expiration. Will update again with corrected log output when available.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 30: printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss);
Comment above added in error, 0x200 refers to PS_HOLD_STATUS not WDOG expiration. […]
Hmm... sorry, I updated to the newest kernel, still does nothing for me. It doesn't seem to have KGDB enabled:
localhost ~ # zcat /proc/config.gz | grep KGDB CONFIG_HAVE_ARCH_KGDB=y # CONFIG_KGDB is not set localhost ~ # grep R87 /etc/lsb-release CHROMEOS_RELEASE_BUILDER_PATH=trogdor-release/R87-13446.0.0 localhost ~ # dmesg | grep "Linux version" [ 0.000000] Linux version 5.4.61-07894-g8809b414e5aa (chrome-bot@chromeos-ci-legacy-us-central2-d-x32-73-coyy) (Chromium OS 11.0_pre394483_p20200618-r11 clang version 11.0.0 (/var/cache/chromeos-cache/distfiles/egit-src/llvm-project b726d071b4aa46004228fc38ee5bfd167f999bfe)) #1 SMP PREEMPT Wed Sep 2 01:48:30 PDT 2020
After the watchdog trigger procedure described above I still get this on boot:
SOC: AOSS_CC_RESET_STATUS : 0 AOSS_CC_RESET_STATUS[c2f0020]..[0]
I'm using a Lazor-rev0. Is there any reason this would only work on a rev1?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/3/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/3/src/soc/qualcomm/sc7180/soc... PS3, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
Hello build bot (Jenkins), mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44868
to look at the new patch set (#4).
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
trogdor: report hardware watchdog reset after reboot
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M src/soc/qualcomm/sc7180/soc.c 1 file changed, 13 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/4
Hello build bot (Jenkins), mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44868
to look at the new patch set (#5).
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
trogdor: report hardware watchdog reset after reboot
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 --- M src/soc/qualcomm/sc7180/soc.c 1 file changed, 13 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/5/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/5/src/soc/qualcomm/sc7180/soc... PS5, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: trogdor: report hardware watchdog reset after reboot ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/6/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/6/src/soc/qualcomm/sc7180/soc... PS6, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
Hello build bot (Jenkins), mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44868
to look at the new patch set (#7).
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
sc7180 : report hardware watchdog reset after reboot
add WATCHDOG_TOMBSTONE in memlayout.ld
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 --- M src/soc/qualcomm/sc7180/Makefile.inc M src/soc/qualcomm/sc7180/include/soc/clock.h A src/soc/qualcomm/sc7180/include/soc/watchdog.h M src/soc/qualcomm/sc7180/memlayout.ld M src/soc/qualcomm/sc7180/soc.c A src/soc/qualcomm/sc7180/watchdog.c 6 files changed, 48 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
Patch Set 7:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/soc... PS7, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... File src/soc/qualcomm/sc7180/watchdog.c:
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 11: uint32_t wdog_sta; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 11: uint32_t wdog_sta; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 12: wdog_sta = read32(&aoss->aoss_cc_reset_status); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 12: wdog_sta = read32(&aoss->aoss_cc_reset_status); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/7/src/soc/qualcomm/sc7180/wat... PS7, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
Patch Set 8:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/soc... PS8, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... File src/soc/qualcomm/sc7180/watchdog.c:
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 11: uint32_t wdog_sta; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 11: uint32_t wdog_sta; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 12: wdog_sta = read32(&aoss->aoss_cc_reset_status); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 12: wdog_sta = read32(&aoss->aoss_cc_reset_status); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/8/src/soc/qualcomm/sc7180/wat... PS8, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
Patch Set 9:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/soc... PS9, Line 32: if(aoss & WDOG_RESET_BIT_MASK) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... File src/soc/qualcomm/sc7180/watchdog.c:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 11: uint32_t wdog_sta; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 11: uint32_t wdog_sta; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 12: wdog_sta = read32(&aoss->aoss_cc_reset_status); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 12: wdog_sta = read32(&aoss->aoss_cc_reset_status); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); line over 96 characters
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); please, no spaces at the start of a line
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
Patch Set 9:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/clock.h:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... PS9, Line 134: u32 aoss_cc_apcs_misc; Might be good to add a check_member() for this one.
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/watchdog.h:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... PS9, Line 8: #endif // _SOC_QUALCOMM_SC7180_WDOG_H__ Please use /* C89 comments */
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/soc... PS9, Line 27: volatile unsigned int aoss = *(unsigned int *)AOSS_CC_RESET_STATUS; Everything in this file is now no longer used and should be deleted, right?
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... File src/soc/qualcomm/sc7180/watchdog.c:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 11: uint32_t wdog_sta;
code indent should use tabs where possible
Please fix all these formatting warnings.
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK); The eventlog code already prints a message when it's logging the event so this doesn't really add anything else, I don't think we need it.
mturney mturney has uploaded a new patch set (#10) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
sc7180 : report hardware watchdog reset after reboot
add WATCHDOG_TOMBSTONE in memlayout.ld
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 --- M src/soc/qualcomm/sc7180/Makefile.inc M src/soc/qualcomm/sc7180/include/soc/clock.h A src/soc/qualcomm/sc7180/include/soc/watchdog.h M src/soc/qualcomm/sc7180/memlayout.ld A src/soc/qualcomm/sc7180/watchdog.c 5 files changed, 32 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/10
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/clock.h:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... PS9, Line 134: u32 aoss_cc_apcs_misc;
Might be good to add a check_member() for this one.
I'm not sure what this means or is in reference to, so will let Ravi handle this input.
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/watchdog.h:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... PS9, Line 8: #endif // _SOC_QUALCOMM_SC7180_WDOG_H__
Please use /* C89 comments */
Done
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/soc... PS9, Line 27: volatile unsigned int aoss = *(unsigned int *)AOSS_CC_RESET_STATUS;
Everything in this file is now no longer used and should be deleted, right?
Done
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... File src/soc/qualcomm/sc7180/watchdog.c:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 11: uint32_t wdog_sta;
Please fix all these formatting warnings.
Done
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/wat... PS9, Line 14: printk(BIOS_INFO, "\nWDOG: Last reset was AOSS_CC_RESET_STATUS : %x\n", wdog_sta & WDOG_RESET_BIT_MASK);
The eventlog code already prints a message when it's logging the event so this doesn't really add an […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180 : report hardware watchdog reset after reboot ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44868/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44868/10//COMMIT_MSG@7 PS10, Line 7: sc7180 : Please remove space before the colon.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44868
to look at the new patch set (#11).
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
sc7180: report hardware watchdog reset after reboot
add WATCHDOG_TOMBSTONE in memlayout.ld
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 --- M src/soc/qualcomm/sc7180/Makefile.inc M src/soc/qualcomm/sc7180/include/soc/clock.h A src/soc/qualcomm/sc7180/include/soc/watchdog.h M src/soc/qualcomm/sc7180/memlayout.ld A src/soc/qualcomm/sc7180/watchdog.c 5 files changed, 34 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/11
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44868/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44868/10//COMMIT_MSG@7 PS10, Line 7: sc7180 :
Please remove space before the colon.
Done
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/clock.h:
https://review.coreboot.org/c/coreboot/+/44868/9/src/soc/qualcomm/sc7180/inc... PS9, Line 134: u32 aoss_cc_apcs_misc;
I'm not sure what this means or is in reference to, so will let Ravi handle this input.
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
Patch Set 12: Code-Review+2
(2 comments)
LGTM after comments
https://review.coreboot.org/c/coreboot/+/44868/12//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44868/12//COMMIT_MSG@11 PS12, Line 11: Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 This needs a Signed-off-by line
https://review.coreboot.org/c/coreboot/+/44868/12/src/soc/qualcomm/sc7180/in... File src/soc/qualcomm/sc7180/include/soc/clock.h:
https://review.coreboot.org/c/coreboot/+/44868/12/src/soc/qualcomm/sc7180/in... PS12, Line 308: void clock_last_reset_was_watchdog(void); Doesn't belong here anymore.
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44868/12//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44868/12//COMMIT_MSG@11 PS12, Line 11: Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119
This needs a Signed-off-by line
Done
https://review.coreboot.org/c/coreboot/+/44868/12/src/soc/qualcomm/sc7180/in... File src/soc/qualcomm/sc7180/include/soc/clock.h:
https://review.coreboot.org/c/coreboot/+/44868/12/src/soc/qualcomm/sc7180/in... PS12, Line 308: void clock_last_reset_was_watchdog(void);
Doesn't belong here anymore.
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44868
to look at the new patch set (#13).
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
sc7180: report hardware watchdog reset after reboot
add WATCHDOG_TOMBSTONE in memlayout.ld
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M src/soc/qualcomm/sc7180/Makefile.inc M src/soc/qualcomm/sc7180/include/soc/clock.h A src/soc/qualcomm/sc7180/include/soc/watchdog.h M src/soc/qualcomm/sc7180/memlayout.ld A src/soc/qualcomm/sc7180/watchdog.c 5 files changed, 33 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/44868/13
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
Patch Set 13: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
Patch Set 13:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44868/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44868/1//COMMIT_MSG@8 PS1, Line 8:
This is missing a commit message body and a Signed-off-by: line, btw. […]
Done
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 27: volatile unsigned int aoss = *(unsigned int *)AOSS_CC_RESET_STATUS;
This was put in place by Ravi and T.mike who are not driver developers as a quick test. […]
Ack
https://review.coreboot.org/c/coreboot/+/44868/1/src/soc/qualcomm/sc7180/soc... PS1, Line 30: printk(BIOS_INFO, "AOSS_CC_RESET_STATUS[%x]..[%x]\n", AOSS_CC_RESET_STATUS, aoss);
Hmm... sorry, I updated to the newest kernel, still does nothing for me. […]
Issue was resolved by moving to romstage.
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
sc7180: report hardware watchdog reset after reboot
add WATCHDOG_TOMBSTONE in memlayout.ld
Change-Id: I57ece39ff3d49f2bab259cbd92ab039a49323119 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/44868 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/soc/qualcomm/sc7180/Makefile.inc M src/soc/qualcomm/sc7180/include/soc/clock.h A src/soc/qualcomm/sc7180/include/soc/watchdog.h M src/soc/qualcomm/sc7180/memlayout.ld A src/soc/qualcomm/sc7180/watchdog.c 5 files changed, 33 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/soc/qualcomm/sc7180/Makefile.inc b/src/soc/qualcomm/sc7180/Makefile.inc index eea38d9..a0d3bc6 100644 --- a/src/soc/qualcomm/sc7180/Makefile.inc +++ b/src/soc/qualcomm/sc7180/Makefile.inc @@ -29,6 +29,7 @@
################################################################################ romstage-y += cbmem.c +romstage-y += watchdog.c romstage-y += timer.c romstage-y += ../common/qclib.c romstage-y += qclib.c diff --git a/src/soc/qualcomm/sc7180/include/soc/clock.h b/src/soc/qualcomm/sc7180/include/soc/clock.h index 62e2a34..b303efe 100644 --- a/src/soc/qualcomm/sc7180/include/soc/clock.h +++ b/src/soc/qualcomm/sc7180/include/soc/clock.h @@ -28,6 +28,8 @@ #define AOP_RESET_SHFT 0 #define RCG_MODE_DUAL_EDGE 2
+#define WDOG_RESET_BIT_MASK 1 + #define SCALE_FREQ_SHFT 11
struct sc7180_clock { @@ -125,9 +127,13 @@ check_member(sc7180_gcc, apcs_clk_br_en1, 0x52008);
struct sc7180_aoss { - u8 _res[0x5002c]; + u8 _res0[0x50020]; + u32 aoss_cc_reset_status; + u8 _res1[0x5002C - 0x50024]; u32 aoss_cc_apcs_misc; }; +check_member(sc7180_aoss, aoss_cc_reset_status, 0x50020); +check_member(sc7180_aoss, aoss_cc_apcs_misc, 0x5002C);
struct sc7180_disp_cc { u8 _res0[0x2004]; diff --git a/src/soc/qualcomm/sc7180/include/soc/watchdog.h b/src/soc/qualcomm/sc7180/include/soc/watchdog.h new file mode 100644 index 0000000..c5ddb55 --- /dev/null +++ b/src/soc/qualcomm/sc7180/include/soc/watchdog.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _SOC_QUALCOMM_SC7180_WDOG_H__ +#define _SOC_QUALCOMM_SC7180_WDOG_H__ + +void check_wdog(void); + +#endif /* _SOC_QUALCOMM_SC7180_WDOG_H__ */ diff --git a/src/soc/qualcomm/sc7180/memlayout.ld b/src/soc/qualcomm/sc7180/memlayout.ld index 94300e8..0142242 100644 --- a/src/soc/qualcomm/sc7180/memlayout.ld +++ b/src/soc/qualcomm/sc7180/memlayout.ld @@ -27,7 +27,8 @@ SSRAM_END(0x146AE000)
BSRAM_START(0x14800000) - REGION(pbl_timestamps, 0x14800000, 84K, 4K) + REGION(pbl_timestamps, 0x14800000, 83K, 4K) + WATCHDOG_TOMBSTONE(0x14814FFC, 4) BOOTBLOCK(0x14815000, 40K) PRERAM_CBFS_CACHE(0x1481F000, 70K) PRERAM_CBMEM_CONSOLE(0x14830800, 32K) diff --git a/src/soc/qualcomm/sc7180/watchdog.c b/src/soc/qualcomm/sc7180/watchdog.c new file mode 100644 index 0000000..954f68a --- /dev/null +++ b/src/soc/qualcomm/sc7180/watchdog.c @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <soc/watchdog.h> +#include <soc/clock.h> +#include <console/console.h> +#include <device/mmio.h> +#include <vendorcode/google/chromeos/chromeos.h> + +void check_wdog(void) +{ + uint32_t wdog_sta = read32(&aoss->aoss_cc_reset_status); + + if (wdog_sta & WDOG_RESET_BIT_MASK) + mark_watchdog_tombstone(); +}
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44868 )
Change subject: sc7180: report hardware watchdog reset after reboot ......................................................................
Patch Set 14:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19479 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19478 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/19477 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19476 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/19475 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19483 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19482 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19481 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19480
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
build bot (Jenkins) (Code Review)
-
Julius Werner (Code Review)
-
mturney mturney (Code Review)
-
Paul Menzel (Code Review)
-
Ravi kumar (Code Review)
-
Ravi Kumar Bokka (Code Review)