Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions sound/soc/sof/intel/hda.c
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,8 @@ static struct snd_soc_acpi_adr_device *find_acpi_adr_device(struct device *dev,
return adr_dev;
}

#define ENUMERATION_TIMEOUT 3000
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

ENUMERATION_TIMEOUT is a very generic macro name and doesn’t document the unit. Rename it to something scoped (e.g. SOF_SDW_ENUMERATION_TIMEOUT_MS) and/or add a _MS suffix so it’s clear this value is in milliseconds.

Suggested change
#define ENUMERATION_TIMEOUT 3000
#define SOF_SDW_ENUMERATION_TIMEOUT_MS 3000

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

ENUMERATION_TIMEOUT is a very generic macro name and the units aren’t obvious at the definition site. Since it’s passed to msecs_to_jiffies(), consider renaming to something like HDA_SDW_ENUMERATION_TIMEOUT_MS (or adding a short comment) to make the units and scope clear and reduce the risk of collisions.

Suggested change
#define ENUMERATION_TIMEOUT 3000
#define ENUMERATION_TIMEOUT 3000 /* milliseconds, used with msecs_to_jiffies() for HDA SoundWire enumeration */

Copilot uses AI. Check for mistakes.

static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev)
{
struct snd_sof_pdata *pdata = sdev->pdata;
Expand All @@ -1315,7 +1317,9 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev
struct sdw_peripherals *peripherals;
struct snd_soc_acpi_mach *mach;
struct sof_intel_hda_dev *hdev;
struct sdw_slave *slave;
int link_index, link_num;
unsigned long time;
int amp_index = 1;
u32 link_mask = 0;
int i;
Expand Down Expand Up @@ -1418,9 +1422,21 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev

/* Generate snd_soc_acpi_link_adr struct for each peripheral reported by the ACPI table */
for (i = 0; i < peripherals->num_peripherals; i++) {
slave = peripherals->array[i];

/* Check if the SoundWire peripheral is present */
if (sof_debug_check_flag(SOF_DBG_CHECK_SDW_PERIPHERAL) && !slave->dev_num_sticky) {
/* Wait for the peripheral to enumerate */
time = wait_for_completion_timeout(&slave->enumeration_complete,
msecs_to_jiffies(ENUMERATION_TIMEOUT));
if (!time) {
dev_warn(&slave->dev, "SoundWire peripheral is not present\n");
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

The warning message is too generic to diagnose which ACPI-described peripheral was missing. Include identifying information (e.g. link_id, mfg_id/part_id/unique_id or dev_name) so users can correlate the warning with the ACPI table entry.

Suggested change
dev_warn(&slave->dev, "SoundWire peripheral is not present\n");
dev_warn(&slave->dev,
"SoundWire peripheral is not present on link %d (dev %s)\n",
slave->bus->link_id, dev_name(&slave->dev));

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The device information is already included in slave->dev.

continue;
}
}
/* link_index = the number of used links below the current link */
link_index = hweight32(link_mask & (BIT(peripherals->array[i]->bus->link_id) - 1));
links[link_index].adr_d = find_acpi_adr_device(sdev->dev, peripherals->array[i],
link_index = hweight32(link_mask & (BIT(slave->bus->link_id) - 1));
links[link_index].adr_d = find_acpi_adr_device(sdev->dev, slave,
&links[link_index], &amp_index);
if (!links[link_index].adr_d)
return NULL;
Expand Down
3 changes: 3 additions & 0 deletions sound/soc/sof/sof-priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ struct snd_sof_pcm_stream;
* DSPLESS_MODE is not set.
* No audio functionality when enabled.
*/
#define SOF_DBG_CHECK_SDW_PERIPHERAL BIT(17) /* Check if SoundWire peripherals are
* present while selecting machine driver
*/

/* Flag definitions used for controlling the DSP dump behavior */
#define SOF_DBG_DUMP_REGS BIT(0)
Expand Down
Loading