ASoC: soc_sdw_utils: skip aux device if it is not present#5697
ASoC: soc_sdw_utils: skip aux device if it is not present#5697mstrozek wants to merge 2 commits intothesofproject:topic/sof-devfrom
Conversation
mstrozek
requested review from
bardliao,
dbaluta,
kv2019i,
lgirdwood,
plbossart,
ranj063 and
ujfalusi
as code owners
|
Hi @bardliao can I ask for a review of this please? |
Approved. Sorry for the delay. I overlooked the PR. |
There was a problem hiding this comment.
Pull request overview
This PR updates the SoundWire ASoC helper logic to avoid creating/counting SDCA aux devices (e.g., HID) when the corresponding SDCA function is not reported by the peripheral, mirroring existing “skip endpoint if not present” behavior.
Changes:
- Add
is_sdca_aux_dev_present()to detect whether an SDCA aux function exists on a SoundWire slave. - Use SDCA function presence to (a) count aux devices more accurately and (b) skip creating aux dev entries when absent.
- Convert the
is_sdca_endpoint_present()header comment to kernel-doc style and clarify its return semantics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * is_sdca_aux_dev_present - Check if an SDCA aux device is present on the SDW peripheral | ||
| * @dev: Device pointer | ||
| * @aux_codec_name: Aux codec name from the codec info (e.g. "snd_soc_sdca.HID.2") |
There was a problem hiding this comment.
The example aux codec name here ("snd_soc_sdca.HID.2") doesn’t match how aux codec names are defined in this file (e.g. codec_info_list uses "snd_soc_sdca.HID"). Please update the example/description to reflect the actual expected format.
| * @aux_codec_name: Aux codec name from the codec info (e.g. "snd_soc_sdca.HID.2") | |
| * @aux_codec_name: Aux codec name from the codec info (e.g. "snd_soc_sdca.HID") |
| sdw_dev = bus_find_device_by_name(&sdw_bus_type, NULL, sdw_codec_name); | ||
| if (!sdw_dev) { | ||
| dev_err(dev, "codec %s not found\n", sdw_codec_name); | ||
| return -EINVAL; | ||
| } |
There was a problem hiding this comment.
When the SoundWire codec device isn’t found on the bus, returning -EINVAL will fail the machine driver probe permanently. For probe ordering races this should typically return -EPROBE_DEFER (and ideally avoid dev_err noise), consistent with other sdw_utils users of bus_find_device_by_name (e.g. sound/soc/sdw_utils/soc_sdw_rt711.c:148-151).
| slave = dev_to_sdw_dev(sdw_dev); | ||
|
|
||
| if (!slave->sdca_data.interface_revision) { | ||
| dev_warn(dev, "No SDCA properties, assuming aux '%s' present\n", aux_codec_name); |
There was a problem hiding this comment.
This warning is emitted against the card device; using &slave->dev would be more consistent with is_sdca_endpoint_present() (which reports SDCA property issues on the slave device) and makes logs easier to attribute to the right peripheral.
| dev_warn(dev, "No SDCA properties, assuming aux '%s' present\n", aux_codec_name); | |
| dev_warn(&slave->dev, "No SDCA properties, assuming aux '%s' present\n", | |
| aux_codec_name); |
sound/soc/sdw_utils/soc_sdw_utils.c
Outdated
| struct snd_soc_component *component; | ||
|
|
||
| ret = is_sdca_aux_dev_present(dev, codec_info->auxs[j].codec_name, | ||
| adr_link, i); |
sound/soc/sdw_utils/soc_sdw_utils.c
Outdated
|
|
||
| if (ret == 0) { | ||
| dev_dbg(dev, "Aux '%s' skipped (SDCA not present)\n", | ||
| codec_info->auxs[j].codec_name); |
sound/soc/sdw_utils/soc_sdw_utils.c
Outdated
| *num_aux += codec_info->aux_num; | ||
| for (j = 0; j < codec_info->aux_num; j++) { | ||
| ret = is_sdca_aux_dev_present(dev, codec_info->auxs[j].codec_name, | ||
| adr_link, i); |
sound/soc/sdw_utils/soc_sdw_utils.c
Outdated
| return ret; | ||
|
|
||
| if (ret == 0) { | ||
| dev_dbg(dev, "Aux '%s' skipped (SDCA not present)\n", |
There was a problem hiding this comment.
you print about skipping SDCA thing here, but not in asoc_sdw_count_sdw_endpoints() path, but in both path you do print if it is there?
For a casual observer it might look that suddenly an aux device appeared (or disappeared, depending on the call order) without SDCA mark.
There was a problem hiding this comment.
ok yes, a bit awkward, deleted it here
Similarly to codec endpoints that may not be used [1], aux devices (like HID) also may not be used. Hence skip aux devices which are not present. [1] https://lore.kernel.org/all/20250414063239.85200-12-yung-chuan.liao@linux.intel.com/ Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Update the comment above is_sdca_endpoint_present(). Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
ASoC: soc_sdw_utils: skip aux device if it is not present
Similarly to codec endpoints that may not be used [1], aux devices (like
HID) also may not be used. Hence skip aux devices which are not present.
Also update the comment above is_sdca_endpoint_present().
[1] https://lore.kernel.org/all/20250414063239.85200-12-yung-chuan.liao@linux.intel.com/