SoundWire fails because of trying program BUSCLOCK_SCALE on devices that were not enumerate

[rfvirgil]

The UpXtreme i14 (MTL) reports in ACPI that it has a RT711 on SoundWire bus 0, but it does NOT have a RT711. (Apparently this is quite common for motherboard ACPI to report a RT711 that is not really there).

The patch

Soundwire: stream: program BUSCLOCK_SCALE
    
    We need to program bus clock scale to adjust the bus clock if current
    bus clock doesn't fit the bandwidth.

Tries to program this RT711 that does not really exist on the bus, and then throws a lot of errors like:

[ 3783.196648] rt711-sdca sdw:0:0:025d:0711:01: SDW_SCP_BUSCLOCK_SCALE register write failed [ 3783.196998] soundwire sdw-master-0-0: Program params failed: -61 [ 3783.197028] SDW1-Playback-SmartAmp: ASoC: error at snd_soc_link_prepare on SDW1-Playback-SmartAmp: -61 [ 3783.197088] SDW1-Playback-SmartAmp: ASoC: error at __soc_pcm_prepare on SDW1-Playback-SmartAmp: -61 [ 3783.197119] Speaker: ASoC: error at dpcm_be_dai_prepare on Speaker: -61 [ 3783.197145] Speaker: ASoC: error at dpcm_fe_dai_prepare on Speaker: -61 [ 3783.203646] sdw_deprepare_stream: subdevice #0-Playback: inconsistent state state 1

If I partially revert that patch (but leave the new function is_clock_scaling_supported_by_slave() because later patches call it) it fixes this problem.

[bardliao]

I can reproduce the issue. But even I reverted the change in sdw_program_params(), I will get below errors

[   89.142991] rt711-sdca sdw:0:0:025d:0711:01: DPN_PortCtrl register write failed for port 3
[   89.142996] soundwire sdw-master-0-0: Program transport params failed: -61
[   89.142998] soundwire sdw-master-0-0: Program params failed: -61
[   89.143000]  SDW0-Playback: ASoC: error at snd_soc_link_prepare on SDW0-Playback: -61
[   89.143003]  SDW0-Playback: ASoC: error at __soc_pcm_prepare on SDW0-Playback: -61
[   89.143006]  Jack Out: ASoC: error at dpcm_be_dai_prepare on Jack Out: -61
[   89.143008]  Jack Out: ASoC: error at dpcm_fe_dai_prepare on Jack Out: -61

I think that is a valid issue when we try to access an unexist codec. I guess that you didn't meet the issue because you were testing with Speaker and the rt711 is not one of the Slave runtime. Can you check if below change works for you?

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 50ebeb048e6e..e933407a3f0f 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -643,6 +643,8 @@ static int sdw_program_params(struct sdw_bus *bus, bool prepare)

        /* Check if all Peripherals comply with SDCA */
        list_for_each_entry(slave, &bus->slaves, node) {
+               if (slave->status == SDW_SLAVE_UNATTACHED)
+                       continue;
                if (!is_clock_scaling_supported_by_slave(slave)) {
                        dev_dbg(&slave->dev, "The Peripheral doesn't comply with SDCA\n");
                        goto manager_runtime;
@@ -659,6 +661,8 @@ static int sdw_program_params(struct sdw_bus *bus, bool prepare)
                int scale_index;
                u8 base;

+               if (slave->status == SDW_SLAVE_UNATTACHED)
+                       continue;
                scale_index = sdw_slave_get_scale_index(slave, &base);
                if (scale_index < 0)
                        return scale_index;

Thanks.

[plbossart]

@bardliao probably best to avoid testing a status. I'd check if the device has ever been enumerated with the sticky device num. I think we have other cases where we check for 'ghost' devices, don't we?

[bardliao]

Welcome back @plbossart. I forgot how did we handle with the 'ghost' devices. @rfvirgil Can you try below change?

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 50ebeb048e6e..d4d69ece7969 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -643,6 +643,8 @@ static int sdw_program_params(struct sdw_bus *bus, bool prepare)

        /* Check if all Peripherals comply with SDCA */
        list_for_each_entry(slave, &bus->slaves, node) {
+               if (!slave->dev_num_sticky)
+                       continue;
                if (!is_clock_scaling_supported_by_slave(slave)) {
                        dev_dbg(&slave->dev, "The Peripheral doesn't comply with SDCA\n");
                        goto manager_runtime;
@@ -659,6 +661,8 @@ static int sdw_program_params(struct sdw_bus *bus, bool prepare)
                int scale_index;
                u8 base;

+               if (!slave->dev_num_sticky)
+                       continue;
                scale_index = sdw_slave_get_scale_index(slave, &base);
                if (scale_index < 0)
                        return scale_index;

[rfvirgil]

@bardliao Sorry, I didn't see your requests for me to test changes. I don't seem to have received a notification email from github. I will test those and let you know.

[rfvirgil]

@bardliao Your second patch (test dev_num_sticky) fixes the problem for me, and the other peripherals on the bus are still working

[rfvirgil]

@bardliao The fixed patch is in 6.14 so this ticket can be closed.