* [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present
2026-09-15 13:13 [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Bard Liao
@ 2026-09-15 13:13 ` Bard Liao
2026-09-15 17:32 ` Cezary Rojewski
2026-09-15 13:13 ` [PATCH 2/3] soundwire: change sdw_show_ping_status type to int Bard Liao
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Bard Liao @ 2026-09-15 13:13 UTC (permalink / raw)
To: linux-sound, vkoul, broonie, tiwai
Cc: vinod.koul, linux-kernel, pierre-louis.bossart, peter.ujfalusi,
bard.liao
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>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
drivers/soundwire/bus.c | 7 +++++++
include/linux/soundwire/sdw.h | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index aeaae5a57c89..b49864ca683a 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -161,7 +161,13 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
bus->params.curr_dr_freq = bus->params.max_dr_freq;
bus->params.curr_bank = SDW_BANK0;
bus->params.next_bank = SDW_BANK1;
+ /*
+ * Set is_present = true by default. It will be set to false when no peripherals
+ * are attached on the bus.
+ */
+ bus->is_present = true;
+ init_completion(&bus->enumeration_complete);
return 0;
}
EXPORT_SYMBOL(sdw_bus_master_add);
@@ -847,6 +853,7 @@ static int sdw_program_device_num(struct sdw_bus *bus, bool *programmed)
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) {
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f710e5932b4b..0cb7e4fef00e 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1001,6 +1001,8 @@ struct sdw_stream_runtime {
* transport and port parameters
* @defer_msg: Defer message
* @params: Current bus parameters
+ * @enumeration_complete: completion utility to control potential races between
+ * enumeration completion and peripheral presence checks.
* @stream_refcount: number of streams currently using this bus
* @bpt_stream_refcount: number of BTP streams currently using this bus (should
* be zero or one, multiple streams per link is not supported).
@@ -1029,6 +1031,7 @@ struct sdw_stream_runtime {
* are supported. This flag is populated by drivers after reading
* appropriate firmware (ACPI/DT).
* @lane_used_bandwidth: how much bandwidth in bits per second is used by each lane
+ * @is_present: indicates whether any peripheral is present on the bus.
*/
struct sdw_bus {
struct device *dev;
@@ -1042,6 +1045,7 @@ struct sdw_bus {
struct list_head m_rt_list;
struct sdw_defer defer_msg;
struct sdw_bus_params params;
+ struct completion enumeration_complete;
int stream_refcount;
int bpt_stream_refcount;
struct sdw_stream_runtime *bpt_stream;
@@ -1064,6 +1068,7 @@ struct sdw_bus {
#endif
bool multi_link;
unsigned int lane_used_bandwidth[SDW_MAX_LANES];
+ bool is_present;
};
struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name, enum sdw_stream_type type);
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present
2026-09-15 13:13 ` [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present Bard Liao
@ 2026-09-15 17:32 ` Cezary Rojewski
2026-09-15 18:31 ` Pierre-Louis Bossart
0 siblings, 1 reply; 9+ messages in thread
From: Cezary Rojewski @ 2026-09-15 17:32 UTC (permalink / raw)
To: Bard Liao
Cc: vinod.koul, linux-kernel, pierre-louis.bossart, peter.ujfalusi,
bard.liao, linux-sound, vkoul, broonie, tiwai
On 9/15/2026 3:13 PM, Bard Liao wrote:
> 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>
> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
> drivers/soundwire/bus.c | 7 +++++++
> include/linux/soundwire/sdw.h | 5 +++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index aeaae5a57c89..b49864ca683a 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -161,7 +161,13 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
> bus->params.curr_dr_freq = bus->params.max_dr_freq;
> bus->params.curr_bank = SDW_BANK0;
> bus->params.next_bank = SDW_BANK1;
> + /*
> + * Set is_present = true by default. It will be set to false when no peripherals
> + * are attached on the bus.
> + */
> + bus->is_present = true;
Does the approach permit existence of non-ghost, SDW master instance
with no codecs attached?
> + init_completion(&bus->enumeration_complete);
> return 0;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present
2026-09-15 17:32 ` Cezary Rojewski
@ 2026-09-15 18:31 ` Pierre-Louis Bossart
2026-09-16 2:50 ` Liao, Bard
0 siblings, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2026-09-15 18:31 UTC (permalink / raw)
To: Cezary Rojewski, Bard Liao
Cc: vinod.koul, linux-kernel, peter.ujfalusi, bard.liao, linux-sound,
vkoul, broonie, tiwai
On 9/15/26 19:32, Cezary Rojewski wrote:
> On 9/15/2026 3:13 PM, Bard Liao wrote:
>> 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>
>> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
>> ---
>> drivers/soundwire/bus.c | 7 +++++++
>> include/linux/soundwire/sdw.h | 5 +++++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
>> index aeaae5a57c89..b49864ca683a 100644
>> --- a/drivers/soundwire/bus.c
>> +++ b/drivers/soundwire/bus.c
>> @@ -161,7 +161,13 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
>> bus->params.curr_dr_freq = bus->params.max_dr_freq;
>> bus->params.curr_bank = SDW_BANK0;
>> bus->params.next_bank = SDW_BANK1;
>> + /*
>> + * Set is_present = true by default. It will be set to false when no peripherals
>> + * are attached on the bus.
>> + */
>> + bus->is_present = true;
>
> Does the approach permit existence of non-ghost, SDW master instance
> with no codecs attached?
Also wondering how the 'mockup' codecs would be handled? That's very
useful to test a manager with no actual codecs attached.
>
>> + init_completion(&bus->enumeration_complete);
>> return 0;
>> }
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present
2026-09-15 18:31 ` Pierre-Louis Bossart
@ 2026-09-16 2:50 ` Liao, Bard
0 siblings, 0 replies; 9+ messages in thread
From: Liao, Bard @ 2026-09-16 2:50 UTC (permalink / raw)
To: Pierre-Louis Bossart, Rojewski, Cezary, Bard Liao
Cc: vinod.koul, linux-kernel, peter.ujfalusi, linux-sound, vkoul,
broonie, tiwai
> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
> Sent: Wednesday, September 16, 2026 2:32 AM
> To: Rojewski, Cezary <cezary.rojewski@intel.com>; Bard Liao <yung-
> chuan.liao@linux.intel.com>
> Cc: vinod.koul@linaro.org; linux-kernel@vger.kernel.org;
> peter.ujfalusi@linux.intel.com; Liao, Bard <bard.liao@intel.com>; linux-
> sound@vger.kernel.org; vkoul@kernel.org; broonie@kernel.org;
> tiwai@suse.de
> Subject: Re: [PATCH 1/3] soundwire: allow drivers to check whether the
> peripheral is present
>
> On 9/15/26 19:32, Cezary Rojewski wrote:
> > On 9/15/2026 3:13 PM, Bard Liao wrote:
> >> 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>
> >> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> >> ---
> >> drivers/soundwire/bus.c | 7 +++++++
> >> include/linux/soundwire/sdw.h | 5 +++++
> >> 2 files changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> >> index aeaae5a57c89..b49864ca683a 100644
> >> --- a/drivers/soundwire/bus.c
> >> +++ b/drivers/soundwire/bus.c
> >> @@ -161,7 +161,13 @@ int sdw_bus_master_add(struct sdw_bus *bus,
> struct device *parent,
> >> bus->params.curr_dr_freq = bus->params.max_dr_freq;
> >> bus->params.curr_bank = SDW_BANK0;
> >> bus->params.next_bank = SDW_BANK1;
> >> + /*
> >> + * Set is_present = true by default. It will be set to false when no
> peripherals
> >> + * are attached on the bus.
> >> + */
> >> + bus->is_present = true;
> >
> > Does the approach permit existence of non-ghost, SDW master instance
> > with no codecs attached?
Sorry, I didn't get your question. What do you mean by non-ghost but
with no codecs attached?
The bus->is_present flag will remain true unless someone to check the
existence of codecs attached on the bus. Currently, Intel SOF driver
will check the existence when it doesn't find the matched configurations
in the mach table like snd_soc_acpi_intel_ptl_sdw_machines[]. So far no
one else will check the existence and bus->is_present will remain true.
>
> Also wondering how the 'mockup' codecs would be handled? That's very
> useful to test a manager with no actual codecs attached.
If the 'mockup' codec configurations are add in the mach table, and they
are added in the ACPI table, then the corresponding machine driver and
monolithic topology will be selected. And we will not check the existence
of codecs in this case. In other words, the mockup support will not
change with the patch set.
> >
> >> + init_completion(&bus->enumeration_complete);
> >> return 0;
> >> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] soundwire: change sdw_show_ping_status type to int
2026-09-15 13:13 [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Bard Liao
2026-09-15 13:13 ` [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present Bard Liao
@ 2026-09-15 13:13 ` Bard Liao
2026-09-15 13:13 ` [PATCH 3/3] ASoC: SOF: Intel: wait and verifies the presence of SoundWire peripherals Bard Liao
2026-09-15 19:08 ` [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Pierre-Louis Bossart
3 siblings, 0 replies; 9+ messages in thread
From: Bard Liao @ 2026-09-15 13:13 UTC (permalink / raw)
To: linux-sound, vkoul, broonie, tiwai
Cc: vinod.koul, linux-kernel, pierre-louis.bossart, peter.ujfalusi,
bard.liao
So that the caller can get the ping results.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
drivers/soundwire/bus.c | 16 +++++++++++-----
include/linux/soundwire/sdw.h | 2 +-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index b49864ca683a..ee90f7d3233e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -314,13 +314,16 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
* sdw_show_ping_status() - Direct report of PING status, to be used by Peripheral drivers
* @bus: SDW bus
* @sync_delay: Delay before reading status
+ *
+ * returns 0 if there is no peripherals attached, 1 if there are peripherals attached
+ * or a negative error code.
*/
-void sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay)
+int sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay)
{
u32 status;
if (!bus->ops->read_ping_status)
- return;
+ return -ENOTSUPP;
/*
* wait for peripheral to sync if desired. 10-15ms should be more than
@@ -335,10 +338,13 @@ void sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay)
mutex_unlock(&bus->msg_lock);
- if (!status)
+ if (!status) {
dev_warn(bus->dev, "%s: no peripherals attached\n", __func__);
- else
- dev_dbg(bus->dev, "PING status: %#x\n", status);
+ return 0;
+ }
+
+ dev_dbg(bus->dev, "PING status: %#x\n", status);
+ return 1;
}
EXPORT_SYMBOL(sdw_show_ping_status);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 0cb7e4fef00e..1b03bfad2e6b 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -901,7 +901,7 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
struct fwnode_handle *fwnode);
void sdw_bus_master_delete(struct sdw_bus *bus);
-void sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay);
+int sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay);
/**
* struct sdw_port_config: Master or Slave Port configuration
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/3] ASoC: SOF: Intel: wait and verifies the presence of SoundWire peripherals
2026-09-15 13:13 [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Bard Liao
2026-09-15 13:13 ` [PATCH 1/3] soundwire: allow drivers to check whether the peripheral is present Bard Liao
2026-09-15 13:13 ` [PATCH 2/3] soundwire: change sdw_show_ping_status type to int Bard Liao
@ 2026-09-15 13:13 ` Bard Liao
2026-09-15 19:08 ` [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Pierre-Louis Bossart
3 siblings, 0 replies; 9+ messages in thread
From: Bard Liao @ 2026-09-15 13:13 UTC (permalink / raw)
To: linux-sound, vkoul, broonie, tiwai
Cc: vinod.koul, linux-kernel, pierre-louis.bossart, peter.ujfalusi,
bard.liao
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>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
sound/soc/sof/intel/hda.c | 47 +++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 4dbba9186b29..e1815bfe358b 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -1304,6 +1304,8 @@ static struct snd_soc_acpi_adr_device *find_acpi_adr_device(struct device *dev,
return adr_dev;
}
+#define SDW_ENUM_TIMEOUT_MS 3000
+
static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev)
{
struct snd_sof_pdata *pdata = sdev->pdata;
@@ -1313,7 +1315,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;
@@ -1415,14 +1419,53 @@ static struct snd_soc_acpi_mach *hda_sdw_machine_select(struct snd_sof_dev *sdev
if (!links)
return NULL;
+ /*
+ * Recalculate the link_mask as a link will be empty if all peripherals on the link are
+ * not enumerated
+ */
+ link_mask = 0;
/* 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];
+
+ if (!slave->bus->is_present)
+ continue;
+
+ if (link_mask & BIT(slave->bus->link_id)) {
+ /*
+ * At least one peripheral is present on the link which means that this
+ * link has already been enumerated
+ */
+ goto skip_wait_link_enumeration;
+ }
+
+ if (sdw_show_ping_status(slave->bus, true) == 0) {
+ /* no peripherals attached on this link */
+ slave->bus->is_present = false;
+ continue;
+ }
+
+ 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;
+ }
+
+skip_wait_link_enumeration:
+ /* Check if the SoundWire peripheral is present */
+ if (!slave->dev_num_sticky) {
+ dev_warn(&slave->dev, "SoundWire peripheral is not present\n");
+ 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], &_index);
if (!links[link_index].adr_d)
return NULL;
+ link_mask |= BIT(slave->bus->link_id);
}
mach->drv_name = "sof_sdw";
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table
2026-09-15 13:13 [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Bard Liao
` (2 preceding siblings ...)
2026-09-15 13:13 ` [PATCH 3/3] ASoC: SOF: Intel: wait and verifies the presence of SoundWire peripherals Bard Liao
@ 2026-09-15 19:08 ` Pierre-Louis Bossart
2026-09-16 4:08 ` Liao, Bard
3 siblings, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2026-09-15 19:08 UTC (permalink / raw)
To: Bard Liao, linux-sound, vkoul, broonie, tiwai
Cc: vinod.koul, linux-kernel, peter.ujfalusi, bard.liao
On 9/15/26 15:13, Bard Liao wrote:
> 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.
I am afraid this raises quite a few opens, such as the 'mockup' support
and delayed enumeration.
A better way to only deal with actual codecs would be to only probe
codec drivers when the codecs report as ATTACHED and get enumerated,
instead of during the ACPI parsing stage.
This is a solution that was discussed a ong time ago, probably circa
2016, during one of the LPC miniconferences, and the direction from
maintainers was that the probe could be used to enable resources (power,
gpio, clocks) that might be required for the hardware codec to become
functional and report as ATTACHED. That's the reason why the probe is
done on all codecs exposed in ACPI, even 'ghost' ones, with an
update_status() callback to the codec driver when the presence of that
codec is detected on the bus.
In practice I am not aware of any codec drivers doing anything with
power/gpio/clocks in the probe stages, at least for ACPI platforms, so
it may be a good time to revisit this direction. SDCA class drivers do
exactly what I described, the subdevices are registered only upon
enumeration, not during ACPI parsing. It's a much simpler design with a
lot fewer potential races.
Problems:
- this would be a very invasive change to sdw_slave_add(), with the
device_register() skipped and moved to the enumeration stage. It'd have
to be opt-in and used only a newer platforms to avoid breaking the
'legacy' devices.
- there is still *nothing* that would tell you that all codec hardware
on a given platform completed the enumeration. You could have a fixed
delay but this would need to be large enough to cover all cases and that
could make the platform boot slower than the current solution - not ideal.
Another option would be to parse the ACPI0018 device, which describes
audio endpoints and makes references to codecs, which could be used to
filter out 'ghost' codecs that don't provide any endpoints. The spec for
this ACPI0018 is not public but could be reverse-engineered from Windows
platforms. The main benefit is that this filtering could be done in the
ACPI parsing stages and not change anything in the probe and startup
sequences.
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table
2026-09-15 19:08 ` [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table Pierre-Louis Bossart
@ 2026-09-16 4:08 ` Liao, Bard
0 siblings, 0 replies; 9+ messages in thread
From: Liao, Bard @ 2026-09-16 4:08 UTC (permalink / raw)
To: Pierre-Louis Bossart, Bard Liao, linux-sound, vkoul, broonie, tiwai
Cc: vinod.koul, linux-kernel, peter.ujfalusi
> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
> Sent: Wednesday, September 16, 2026 3:09 AM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>; linux-
> sound@vger.kernel.org; vkoul@kernel.org; broonie@kernel.org;
> tiwai@suse.de
> Cc: vinod.koul@linaro.org; linux-kernel@vger.kernel.org;
> peter.ujfalusi@linux.intel.com; Liao, Bard <bard.liao@intel.com>
> Subject: Re: [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the
> mach table
>
> On 9/15/26 15:13, Bard Liao wrote:
> > 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.
>
> I am afraid this raises quite a few opens, such as the 'mockup' support
> and delayed enumeration.
The mockup support will not change. We will only check the existance if
no mach item matched. And we can always add the mochup configurations to
the mach table.
And indeed, it will add a little delay in some cases. However, based on
our test, only about 1 ms delay will be added.
>
> A better way to only deal with actual codecs would be to only probe
> codec drivers when the codecs report as ATTACHED and get enumerated,
> instead of during the ACPI parsing stage.
But this doesn't solve our issue. The DAI link will still be created
and the sound card will not probe because the codec driver doesn't probe.
Our target is that the ghost device should not be added in the DAI link.
So that the sound card can probe properly.
> This is a solution that was discussed a ong time ago, probably circa
> 2016, during one of the LPC miniconferences, and the direction from
> maintainers was that the probe could be used to enable resources (power,
> gpio, clocks) that might be required for the hardware codec to become
> functional and report as ATTACHED. That's the reason why the probe is
> done on all codecs exposed in ACPI, even 'ghost' ones, with an
> update_status() callback to the codec driver when the presence of that
> codec is detected on the bus.
>
> In practice I am not aware of any codec drivers doing anything with
> power/gpio/clocks in the probe stages, at least for ACPI platforms, so
> it may be a good time to revisit this direction. SDCA class drivers do
> exactly what I described, the subdevices are registered only upon
> enumeration, not during ACPI parsing. It's a much simpler design with a
> lot fewer potential races.
>
> Problems:
> - this would be a very invasive change to sdw_slave_add(), with the
> device_register() skipped and moved to the enumeration stage. It'd have
> to be opt-in and used only a newer platforms to avoid breaking the
> 'legacy' devices.
I didn't change sdw_slave_add(). All the change in SoundWire driver is to
add a flag and a completion to share the information of whether the
enumeration of the bus is completed.
> - there is still *nothing* that would tell you that all codec hardware
> on a given platform completed the enumeration. You could have a fixed
> delay but this would need to be large enough to cover all cases and that
> could make the platform boot slower than the current solution - not ideal.
Yeah, I set a 3 sec timeout. I believe that is large enough. And the flag
and the completion is to avoid the meaningless waiting. With the
is_present flag, we can ensure a bus will not be waiting for more than 1
time. And with the enumeration_complete completion, we won't wait for the
ghost device to be enumerated.
>
> Another option would be to parse the ACPI0018 device, which describes
> audio endpoints and makes references to codecs, which could be used to
> filter out 'ghost' codecs that don't provide any endpoints. The spec for
> this ACPI0018 is not public but could be reverse-engineered from Windows
> platforms. The main benefit is that this filtering could be done in the
> ACPI parsing stages and not change anything in the probe and startup
> sequences.
Is ACPI0018 reliable? Will it still list the ghost device? The ghost rt722
has all endpoints listed in its DisCo table
^ permalink raw reply [flat|nested] 9+ messages in thread