Skip to content

ASoC/SoundWire: remove ghost peripherals from the mach table - #5910

Open
bardliao wants to merge 3 commits into
thesofproject:topic/sof-devfrom
bardliao:verify-peripherals
Open

ASoC/SoundWire: remove ghost peripherals from the mach table#5910
bardliao wants to merge 3 commits into
thesofproject:topic/sof-devfrom
bardliao:verify-peripherals

Conversation

@bardliao

@bardliao bardliao commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

ACPI may report a ghost SoundWire peripheral. It will cause unexpected error like duplicated links, codec driver can't probe, etc. This series check the presence of SoundWire peripherals and skip the non-existing peripherals.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new presence/enumeration gating can misclassify valid links (and even return a default SDW machine with an empty link_mask), leading to incorrect machine selection and runtime behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR aims to prevent “ghost” ACPI-reported SoundWire peripherals from causing duplicated links and probe failures by validating peripheral presence during SOF HDA SoundWire machine selection.

Changes:

  • Add presence/enumeration checks in hda_sdw_machine_select() to filter out non-enumerated peripherals and recompute the link mask.
  • Change sdw_show_ping_status() to return an int so callers can distinguish “no peripherals” from “not supported”.
  • Introduce sdw_bus fields intended to track enumeration completion and whether any peripheral is present on the bus.
File summaries
File Description
sound/soc/sof/intel/hda.c Adds runtime checks (PING + enumeration completion + dev_num_sticky) to skip ghost peripherals and rebuild link_mask.
include/linux/soundwire/sdw.h Updates sdw_show_ping_status() prototype and adds new sdw_bus fields/documentation.
drivers/soundwire/bus.c Implements the sdw_show_ping_status() return value and initializes/completes new sdw_bus state.
Review details

Suppressed comments (3)

sound/soc/sof/intel/hda.c:1465

  • If all ACPI-reported peripherals are filtered out as non-present, link_mask can end up as 0, but the function still returns a default "sof_sdw" machine. This can cause later code to treat the platform as SoundWire-capable while effectively disabling SoundWire startup. Return NULL when no links remain after filtering.
	mach->drv_name = "sof_sdw";
	mach->mach_params.links = links;
	mach->mach_params.link_mask = link_mask;
	mach->mach_params.platform = dev_name(sdev->dev);

include/linux/soundwire/sdw.h:1034

  • Kerneldoc grammar: "indicates is there any" should be reworded to "indicates whether any".
 * @lane_used_bandwidth: how much bandwidth in bits per second is used by each lane
 * @is_present: indicates is there any peripheral present on the bus or not.

drivers/soundwire/bus.c:318

  • sdw_show_ping_status() changed from void to int but the kerneldoc does not describe the return values; please document the meaning of 1/0 and the -ENOTSUPP error so callers can use it correctly.
/**
 * sdw_show_ping_status() - Direct report of PING status, to be used by Peripheral drivers
 * @bus: SDW bus
 * @sync_delay: Delay before reading status
 */
int sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay)
  • Files reviewed: 3/3 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sound/soc/sof/intel/hda.c
Comment thread drivers/soundwire/bus.c
Comment thread include/linux/soundwire/sdw.h Outdated
Comment thread include/linux/soundwire/sdw.h
Comment thread sound/soc/sof/intel/hda.c
Copilot AI review requested due to automatic review settings September 3, 2026 03:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new bus-level enumeration_complete is not reliably managed across error paths and enumeration cycles, which can lead to spurious timeouts and incorrect presence decisions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

sound/soc/sof/intel/hda.c:1445

  • The timeout warning "No peripheral is present" is misleading: a completion timeout means enumeration didn't finish within the expected window, not necessarily that no peripherals exist. This message should mention an enumeration timeout (and ideally the link id) to avoid misdiagnosis.
		time = wait_for_completion_timeout(&slave->bus->enumeration_complete,
						   msecs_to_jiffies(SDW_ENUM_TIMEOUT_MS));
		if (!time) {
			dev_warn(slave->bus->dev, "No peripheral is present\n");
			slave->bus->is_present = false;
			continue;

drivers/soundwire/bus.c:863

  • bus->enumeration_complete is only completed on the -ENODATA path; if sdw_transfer() fails, waiters (e.g., presence checks) will always time out even though enumeration has already stopped with an error. Complete the bus-level completion on the error path as well to avoid spurious timeouts.
			complete_all(&bus->enumeration_complete);
			break;
		}
		if (ret < 0) {
			dev_err(bus->dev, "DEVID read fail:%d\n", ret);
  • Files reviewed: 3/3 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread drivers/soundwire/bus.c
Comment thread sound/soc/sof/intel/hda.c
Comment thread include/linux/soundwire/sdw.h
@naveen-manohar

naveen-manohar commented Sep 3, 2026

Copy link
Copy Markdown

Thanks @bardliao
Verified at my end, No more GHOSTS !!

when Codec is NOT Attached, get expected Error as NO Peripherals Attached
And Lists audio card only when Codec is Attached as expected.
image

A ghost peripheral may be listed in the ACPI table and we want to skip
it. Add enumeration_complete and is_present in struct sdw_bus{} allow
the driver to wait and check whether a peripheral is present.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Copilot AI review requested due to automatic review settings September 9, 2026 03:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new bus-level enumeration completion is not signaled on enumeration error paths (leading to avoidable timeouts/false “not present” decisions) and the updated return-value API lacks corresponding kerneldoc/has a misleading timeout warning.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

drivers/soundwire/bus.c:864

  • bus->enumeration_complete is only completed on the -ENODATA path. If sdw_transfer() returns an error (which sdw_handle_slave_status() explicitly ignores), waiters can time out and higher layers may incorrectly mark the link as not present.
		if (ret == -ENODATA) { /* end of device id reads */
			dev_dbg(bus->dev, "No more devices to enumerate\n");
			ret = 0;
			complete_all(&bus->enumeration_complete);
			break;
		}
		if (ret < 0) {
			dev_err(bus->dev, "DEVID read fail:%d\n", ret);
			break;
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread drivers/soundwire/bus.c
Comment thread sound/soc/sof/intel/hda.c
So that the caller can get the ping results.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
…rals

Wait and verifies the presence of SoundWire peripherals listed in the
ACPI table. This prevents the system from probing non-existent (ghost)
SoundWire devices.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Copilot AI review requested due to automatic review settings September 9, 2026 06:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The new bus-level enumeration completion is not signaled on enumeration error paths and the SOF selector can still return a SoundWire machine with an empty post-filter link_mask, which can misclassify presence and select the wrong machine driver.

Review details

Suppressed comments (2)

sound/soc/sof/intel/hda.c:1472

  • If all ACPI-reported SoundWire peripherals are filtered out (no buses present or no dev_num_sticky), link_mask remains 0 but the function still returns a default "sof_sdw" machine. This can incorrectly select the SoundWire machine driver even though no real peripherals were found; returning NULL here lets the caller fall back to the generic HDA path.
		link_mask |= BIT(slave->bus->link_id);
	}

	mach->drv_name = "sof_sdw";
	mach->mach_params.links = links;

drivers/soundwire/bus.c:868

  • bus->enumeration_complete is only completed on the -ENODATA (normal end) path. If enumeration aborts due to a DEVID read error, waiters (e.g. SOF hda_sdw_machine_select()) can time out and misclassify a link as having no peripherals. Completing the bus-level completion on this error path avoids the spurious timeout.
		if (ret < 0) {
			dev_err(bus->dev, "DEVID read fail:%d\n", ret);
			break;
		}
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants