search

thesofproject / linux

Linux kernel source tree
Other
91 stars 133 forks source link

ASoC: SOF: Intel: hda-ctrl: add missing WAKE_STS clear #4896

Closed plbossart closed 8 months ago

plbossart commented 8 months ago

For some reason, the programming sequences in the SOF driver do not include a clear of the WAKE_STS bits before resetting the controller.

This clear is not formally required by the HDaudio specification, but was added to harden the snd-hda-reset back in 2007. Adding this sequence back avoids an issue reported by the Intel CI.

Closes: https://github.com/thesofproject/linux/issues/4889

marc-hb commented 8 months ago

In a comment in #4894 you posted more information and SHA1s which really seem to belong to this commit message: https://github.com/thesofproject/linux/pull/4894#issuecomment-2030474135

FWIW the 3rd commit partially reverts https://github.com/thesofproject/linux/commit/6ec3295826d8c11141c970cf3fd68d77d3452adb ("ASoC: SOF: Intel: hda: remove duplicated clear WAKESTS")

I have no idea why this was removed.

It could also be that there's an interference from https://github.com/thesofproject/linux/commit/ae5ff22a884d2aac0908cfd0113d3db633d79ae6 ("ASoC: SOF: Intel: hda-ctrl: only clear WAKESTS for HDaudio codecs"), where in the end we didn't clear all the WAKE_STS fields.

Wow, my brain is officially fried haha.

OK: maybe the last line about your brain is not necessary.

marc-hb commented 8 months ago

I have no idea why this was removed.

This was discussed in

plbossart commented 8 months ago

I don't want to add more details on this patch. We don't really know what the correlation/causation is on LNL, adding more details would likely mislead reviewers.

marc-hb commented 8 months ago

I wonder why even the purely factual, past SHA1s could confuse reviewers but I imagine they can be found with some well placed git blame commands so I hope omitting them only adds a bit overhead.