adsp: ace: ipc: boot: power: watchdog: A set of changes required by the watchdog #53840
Conversation
softwarecki
requested review from
andyross,
dcpleung and
nashif
as code owners
zephyrbot
requested review from
MaureenHelm,
aborisovich,
ceolin,
jxstelter,
kv2019i,
lgirdwood,
lyakh,
marc-hb,
marcinszkudlinski and
tmleman
softwarecki
force-pushed
the
watchdog-fixups
branch
2 times, most recently
from
d81f0cc to
8e69b13
Compare
softwarecki
force-pushed
the
watchdog-fixups
branch
2 times, most recently
from
75eb416 to
ea47c91
Compare
softwarecki
force-pushed
the
watchdog-fixups
branch
from
ea47c91 to
ba275ff
Compare
soc/xtensa/intel_adsp/common/ipc.c
Outdated
There was a problem hiding this comment.
This is an emergency function used to send a notification about watchdog timeout on a primary core. We don't want any timeout here.
Added intel_adsp_ipc_send_message_emergency function that allows to send an ipc message notifying about emergency event, such as watchdog timeout. Signed-off-by: Adrian Warecki <[email protected]>
softwarecki
requested review from
abonislawski,
aborisovich,
marcinszkudlinski,
stephanosio and
tmleman
and removed request for
aborisovich
|
@softwarecki If you plan to get this into v3.3, a bug report is required in order for this PR to be merged during the feature freeze. Please file a bug report issue for the problem that this PR is trying to fix and link the issue to this PR. |
The watchdog is controlled by ll-scheduler and should not be resumed when a core is bringing up. Watchdog pause control code was removed. Signed-off-by: Adrian Warecki <[email protected]>
softwarecki
dismissed stale reviews from aborisovich, tmleman, marcinszkudlinski, and abonislawski
via
e55d530
softwarecki
force-pushed
the
watchdog-fixups
branch
from
2e44b87 to
e55d530
Compare
|
@softwarecki Reminder to file a bug report if you need this in for 3.3 :-] |
| } | ||
| } | ||
|
|
||
| regs->idd = ext_data; |
There was a problem hiding this comment.
is it right ? this is overwriting the value set few lines above for regs->idd (CAVS_V15 build)
There was a problem hiding this comment.
Good question. I made it identical to the intel_adsp_ipc_send_message function, which uses these registers regardless of the platform. I suspect that setting this bit triggers message acknowledgment, and it doesn't need to stay set. Maybe @aborisovich can confirm this?
There was a problem hiding this comment.
Yes it looks like by analogy it should work, I'll look into CAVS15 registers asap.
There was a problem hiding this comment.
Ok, so we have it partially wrong.
IDD register on CAVS1.5 is also DIPCIE register.
DIPCIE register has bit 30 used as done bit. When set, it means that ipc target (host here) had completed the operation. DIPCIE message extension is composed of 29 youngest bits.
So basically in this code, we override DIPCIE bits 30 and 31 using extended message what should not happen.
ext_data should be truncated from the left to contain only 29 youngest bits of the extended message for CAVS1.5.
We can of course assume that application logic (that provides extended message value) had prepared the message in the way that is suitable. Let's say application copied state of IDD 31 and 30 bit to the extended message and then wrote remaining 29 bits content. But this is not a good practice in my opinion.
It works for now so I guess we can merge this PR and provide some fixes in separate PR that would also fix the body of intel_adsp_ipc_send_message function.
Is this ok for you @ceolin ?
10 participants
Fixed non-maskable interrupt handling:
The non-maskable interrupt have no corresponding bit in INTERRUPT and INTENABLE registers so its occurrence cannot be confirmed. Removed the code that checked if the interrupt flag is set.
Added emergency ipc message send function"
Added intel_adsp_ipc_send_message_emergency function that allows to send an ipc message notifying about emergency event, such as watchdog timeout.
Separated a watchdog state from core power:
The watchdog is controlled by ll-scheduler and should not be resumed when a core is bringing up. Watchdog pause control code was removed.