Uh oh!
There was an error while loading. Please reload this page.
soundwire: intel: simplify sync_go sequence - #4153
Conversation
6b251c6 to
f44a72dCompareUh oh!
There was an error while loading. Please reload this page.
In the existing code, the SHIM_SYNC::SYNC_GO bit is set, and the code waits for it to return to zero. That second wait part is just wrong: the SYNC_GO bit is *write-only* so there's no way to know if it's cleared by hardware. The code works because the value for a read-only bit is zero, but that's really just luck. Simplify the sequence to a plain read-modify-write. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
f44a72d to
bc2dde8Compare
ujfalusi
left a comment
There was a problem hiding this comment.
now it makes more sense.
Uh oh!
There was an error while loading. Please reload this page.
plbossart
commented
Jan 25, 2023
additional simplifications are possible, closing for now. |
plbossart
commented
Jan 25, 2023
actually no, it's too complicated to simplify further. |
ranj063
commented
Jan 26, 2023
@plbossart the CML SDW tests are failing with this PR. Is this a known issue or is this because of this PR |
ranj063
commented
Jan 26, 2023
Ok looks like its happening in other PRs too |
plbossart
commented
Jan 26, 2023
yes, that board has severe issues with timeouts in suspend/resume. I don't think it's related to this PR. |
plbossart
commented
Feb 9, 2023
already merged with #4157 |
In the existing code, the SHIM_SYNC::SYNC_GO bit is set, and the code waits for it to return to zero.
That second wait part is just wrong: the SYNC_GO bit is write-only so there's no way to know if it's cleared by hardware. The code works because the value for a read-only bit is zero, but that's really just luck.
Simplify the sequence to a plain read-modify-write.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com