soundwire: fix programing unattached BUSCLOCK_SCALE issue#5666
soundwire: fix programing unattached BUSCLOCK_SCALE issue#5666bardliao wants to merge 2 commits intothesofproject:topic/sof-devfrom
Conversation
bardliao
requested review from
charleskeepax,
plbossart,
ranj063,
rfvirgil,
shumingfan,
simontrimmer and
ujfalusi
|
Tested on a LNL product, this looks good and unblocked from my perspective (Though we're still working through the other troubles) |
ujfalusi
left a comment
ujfalusi
left a comment
There was a problem hiding this comment.
are these two separate issues?
do you want these to be backported to stable? Fixes tag?
Hmm, it could be separated. The first commit is to remove the unnecessary writes to the SDW_SCP_BUSCLOCK_SCALE register, and the second commit is to fix the issue that we may access an unattached peripheral. And yes, it is worth to add a Fixes tag to the second commit. I will also revert the commits order to let the fix commit apply to the stable kernel easier. |
bardliao
force-pushed
the
fix-bus_clock_scale_error
branch
from
4e03732 to
fad14e4
Compare
drivers/soundwire/stream.c
Outdated
| /* Skip the unattached Peripherals */ | ||
| if (!completion_done(&slave->enumeration_complete)) { | ||
| dev_warn(&slave->dev, | ||
| "Not enumerated, skip programing BUSCLOCK_SCALE\n"); |
There was a problem hiding this comment.
@bardliao nit-pick: you have several typos in the commit message and here in the word "programming", in the commit message "unattached", "programmed" and "programming". I am good if you want to fix these before sending upstream.
There was a problem hiding this comment.
Updated, could you please double check if I miss any?
…eral The SDW_SCP_BUSCLOCK_SCALE register will be programmed when the Peripheral is attached. We can and should skip programming the SDW_SCP_BUSCLOCK_SCALE register when the Peripheral is unattached. Fixes: 645291c ("Soundwire: stream: program BUSCLOCK_SCALE") Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Currently, we program bus clock scale index unconditionally. It is not necessary if the new index is the same as the current one. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
bardliao
dismissed stale reviews from ranj063, simontrimmer, and ujfalusi
via
39feb17
bardliao
force-pushed
the
fix-bus_clock_scale_error
branch
from
fad14e4 to
39feb17
Compare
| "Not enumerated, skip programming BUSCLOCK_SCALE\n"); | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
if I follow the commit message "The SDW_SCP_BUSCLOCK_SCALE register will be programmed when the
Peripheral is attached", then why do we even need to configure this register during a stream operation, as done below?
| "Not enumerated, skip programming BUSCLOCK_SCALE\n"); | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
also typo in commit tile "unattaced"
5 participants
We do need to set the same bus clock scale on all Peripherals to make sure all Peripherals are running on the same frequency. But we can skip the unattached Peripherals when programing params. Because the frequency will be set when the Peripheral is attached.