-
Notifications
You must be signed in to change notification settings - Fork 349
zephyr: Set config for resumed DAIs #10376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Can one of the admins verify this patch?
|
kv2019i
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks ok.
Not related to this PR specifically, but @abonislawski @tmleman , having "resume_dais()" in zephyr/lib/cpu.c seems very suprising, I wonder we could find a place that makes more sense for this "special SOF side actions when resuming from D3" functionality.
tmleman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this a regression or a new test scenario?
regression... but the test was failing for another reason before you added suspend/resume functionality... so you missed the regression :/ |
| if (dai_probe(dd->dai->dev) < 0) { | ||
| tr_err(&zephyr_tr, "DAI resume failed, type %d index %d", | ||
| tr_err(&zephyr_tr, "DAI resume failed on probe, type %d index %d", | ||
| dd->dai->type, dd->dai->index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in fact wondering if trying to continue after an error here makes sense? Shouldn't we skip this DAI (using continune) if re-probing fails? Same for the dai_set_config() failure below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider that probe might fail simply because dai_remove failed while entering D3 and a given device is already operational. For example I found the following code in dai_ssp_probe:
if (dai_get_drvdata(dp)) {
return -EEXIST; /* already created */
}
In that case we probably can skip dai_set_config. But should we also skip microphone privacy settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If re-probing fails, setting the config makes no sense and will probably fail itself as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just put dai_set_config under "else". The microphone privacy settings probably should stay as is because, if I understand it correctly, the code already is conditional (if an IRQ is not missed during D3 nothing happens)
5f1a2c3 to
ccdf28c
Compare
After exit from D3 reset NHLT DMIC driver is unable to resume its operation due to lack of configuration. This change adds the missing steps for all types of active DAIs. Signed-off-by: Wojciech Jablonski <[email protected]>
After exit from D3 reset NHLT DMIC driver is unable to resume its operation due to lack of configuration. This change adds the missing steps for all types of active DAIs.