Conversation
The struct sdw_bus * argument to sdw_extract_slave_id() is entirely bogus, and isn't needed for the ID conversion that this helper function performs. Also provide a stub implementation for !CONFIG_SOUNDWIRE so that callers can use if (!IS_REACHABLE(CONFIG_SOUNDWIRE)) style instead of having to be surrounded by #ifdefs. The bus argument was only used to get a struct device * for two dev_dbg() statements. This forced callers to have a valid struct sdw_bus object just to convert an ID from u64 to struct sdw_slave_id. The debug print is now a utility function sdw_debug_log_slave_id() that can be used anywhere and only requires a struct device *. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Add a function to walk the component list and call a callback for each component. This lest the machine driver implement its own component match. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
…ddress Use snd_soc_lookup_component_walk() to find the codec for the SoundWire address instead of trying to guess the name of the codec driver. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Fix the disabled-configuration linkage conflict and include the SoundWire link in component matching.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Adds SoundWire-address-based codec component lookup for ASoC/sof_sdw, replacing codec-name guessing.
Changes:
- Adds callback-based ASoC component walking.
- Matches codecs by SoundWire device identity.
- Refactors SoundWire ID extraction and logging APIs.
| File | Description |
|---|---|
sound/soc/soc-core.c |
Adds component walk API |
sound/soc/sdw_utils/soc_sdw_utils.c |
Implements SoundWire component matching |
include/sound/soc.h |
Declares the traversal API |
include/sound/soc_sdw_utils.h |
Declares the SoundWire lookup helper |
include/linux/soundwire/sdw.h |
Updates SoundWire ID APIs |
drivers/soundwire/slave.c |
Uses revised ID extraction |
drivers/soundwire/qcom.c |
Uses revised ID extraction |
drivers/soundwire/bus.h |
Removes the obsolete declaration |
drivers/soundwire/bus.c |
Refactors ID extraction and logging |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1221
to
+1227
| static inline void sdw_extract_slave_id(u64 addr, struct sdw_slave_id *id) | ||
| { | ||
| } | ||
|
|
||
| static inline void sdw_debug_log_slave_id(struct device *dev, const struct sdw_slave_id *id) | ||
| { | ||
| } |
Comment on lines
+1654
to
+1655
| if (sdw_compare_devid(dev_to_sdw_dev(dev), info->id) != 0) | ||
| return 0; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This series adds a walk function to ASoC so sof_sdw can find the codec driver by matching its SoundWire address instead of trying to guess the name of the codec driver.