RE: [PATCH 2/3] ASoC: sdw_utils: skip endpoints of a peripheral that is not on the bus

[Liao, Bard]

> -----Original Message-----
> From: Sergey Lebedev <lsa.uz@xxxxx>
> Sent: Wednesday, August 5, 2026 7:00 AM
> To: Mark Brown <broonie@xxxxxxxxxx>; Liam Girdwood
> <lgirdwood@xxxxxxxxx>; Jaroslav Kysela <perex@xxxxxxxx>; Takashi Iwai
> <tiwai@xxxxxxxx>; Oder Chiou <oder_chiou@xxxxxxxxxxx>; Bard Liao <yung-
> chuan.liao@xxxxxxxxxxxxxxx>; Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>;
> Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx>; Ranjani Sridharan
> <ranjani.sridharan@xxxxxxxxxxxxxxx>; Pierre-Louis Bossart <pierre-
> louis.bossart@xxxxxxxxx>; Daniel Baluta <daniel.baluta@xxxxxxx>; Vijendar
> Mukunda <Vijendar.Mukunda@xxxxxxx>
> Cc: linux-sound@xxxxxxxxxxxxxxx; sound-open-firmware@xxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH 2/3] ASoC: sdw_utils: skip endpoints of a peripheral that is
> not on the bus
> 
> asoc_sdw_parse_sdw_endpoints() builds DAI links for every endpoint of
> every _ADR entry the firmware declares. If a declared peripheral never
> enumerates, its links are still created and later fail to prepare, which
> takes the whole link down rather than degrading it:
> 
>   sof_sdw sof_sdw: ASoC: error at snd_soc_link_startup on
>     SDW0-Playback-SmartAmp: -61
> 
> The Microsoft Surface Pro 11 (Intel) declares one physical RT1320 twice,
> as two _ADR entries on link 0 differing only in SDCA class id:
> 
>   SWRA  _ADR 0x000030025D132000   class 0
>   SWRB  _ADR 0x000030025D132001   class 1
> 
> Same link, same manufacturer, part and version, same unique id 0. The
> part reports class 1, so only SWRB enumerates. SWRA is a phantom and
> stays UNATTACHED across every boot and every firmware version tested,
> including the November 2025 bundle.

Why not just remove SWRA from the BIOS?

> 
> The existing is_sdca_endpoint_present() check cannot filter it out.
> Setting aside that it is gated on a non-zero class id and the phantom is
> the class-0 entry, the deeper problem is that the BIOS describes both
> entries identically: each declares the same two SDCA functions, so the
> check matches for either. Bus presence is what distinguishes them, so
> test that.
> 
> The check is by nature a runtime one, and its correctness depends on the
> peripheral having enumerated by the time the card probes. That holds
> here: the real device is Attached and the phantom has no device number
> at all whenever this runs. It is a weaker property than the surrounding
> BIOS-driven checks, and a suggestion for something stronger would be
> welcome, but the firmware offers nothing else to key on.
> 
> Signed-off-by: Sergey Lebedev <lsa.uz@xxxxx>
> ---
>  sound/soc/sdw_utils/soc_sdw_utils.c | 46
> +++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c
> b/sound/soc/sdw_utils/soc_sdw_utils.c
> index d8db8fc53..12ca4bdd4 100644
> --- a/sound/soc/sdw_utils/soc_sdw_utils.c
> +++ b/sound/soc/sdw_utils/soc_sdw_utils.c
> @@ -1909,6 +1909,46 @@ int asoc_sdw_get_dai_type(u32 type)
>  }
>  EXPORT_SYMBOL_NS(asoc_sdw_get_dai_type, "SND_SOC_SDW_UTILS");
> 
> +/*
> + * Some firmware describes one physical peripheral with two _ADR entries
> that
> + * differ only in SDCA class id, on the same link and with the same unique id.
> + * Only the entry whose class id matches the part ever enumerates; the other
> is
> + * a phantom. Building DAI links for it fails the whole link rather than
> + * degrading it, so the endpoints have to be skipped.
> + *
> + * This cannot be decided from the BIOS description: on the machine that
> + * prompted this, both entries declare an identical set of SDCA functions, so
> + * is_sdca_endpoint_present() below matches for either. Bus presence is the
> only
> + * thing that distinguishes them.
> + */
> +static bool is_peripheral_attached(struct device *dev,
> +				   const struct snd_soc_acpi_link_adr
> *adr_link,
> +				   int adr_index)
> +{
> +	const char *sdw_codec_name;
> +	struct device *sdw_dev;
> +	struct sdw_slave *slave;
> +	bool attached;
> +
> +	sdw_codec_name = _asoc_sdw_get_codec_name(dev, adr_link,
> adr_index);
> +	if (!sdw_codec_name)
> +		return true;
> +
> +	sdw_dev = bus_find_device_by_name(&sdw_bus_type, NULL,
> sdw_codec_name);
> +	if (!sdw_dev)
> +		return true;
> +
> +	slave = dev_to_sdw_dev(sdw_dev);
> +	attached = slave->status != SDW_SLAVE_UNATTACHED;

The status just means the current state. A Peripheral aka Slave on the
bus could be attached or unattached. We can't use the slave->status to
determine whether a Peripheral is physically on the bus or not.
Checking slave->dev_num_sticky may work. However, there is a timing
issue that the Peripheral could be attached after the check.

> +	if (!attached)
> +		dev_dbg(dev, "%s not present on the bus, skipping its
> endpoints\n",
> +			sdw_codec_name);
> +
> +	put_device(sdw_dev);
> +
> +	return attached;
> +}
> +
>  /**
>   * is_sdca_endpoint_present - Check if an SDCA endpoint is present on the
> SDW peripheral
>   * @dev: Device pointer
> @@ -2065,6 +2105,12 @@ int asoc_sdw_parse_sdw_endpoints(struct
> snd_soc_card *card,
>  				dai_info = &codec_info->dais[adr_end->num];
>  				soc_dai = asoc_sdw_find_dailink(soc_dais,
> adr_end);
> 
> +				/* skip a peripheral that is not on the bus at all
> */
> +				if (!is_peripheral_attached(dev, adr_link, i)) {
> +					(*num_devs)--;
> +					continue;
> +				}
> +
>  				/*
>  				 * quirk should have higher priority than the
> sdca properties
>  				 * in the BIOS. We can't always check the DAI
> quirk because we
> --
> 2.50.1 (Apple Git-155)
> 
>