Commit 72124f07 authored by Richard Fitzgerald's avatar Richard Fitzgerald Committed by Vinod Koul
Browse files

soundwire: bus: Don't exit early if no device IDs were programmed 



Only exit sdw_handle_slave_status() right after calling
sdw_program_device_num() if it actually programmed an ID into at
least one device.

sdw_handle_slave_status() should protect itself against phantom
device #0 ATTACHED indications. In that case there is no actual
device still on #0. The early exit relies on there being a status
change to ATTACHED on the reprogrammed device to trigger another
call to sdw_handle_slave_status() which will then handle the status
of all peripherals. If no device was actually programmed with an
ID there won't be a new ATTACHED indication. This can lead to the
status of other peripherals not being handled.

The status passed to sdw_handle_slave_status() is obviously always
from a point of time in the past, and may indicate accumulated
unhandled events (depending how the bus manager operates). It's
possible that a device ID is reprogrammed but the last PING status
captured state just before that, when it was still reporting on
ID #0. Then sdw_handle_slave_status() is called with this PING info,
just before a new PING status is available showing it now on its new
ID. So sdw_handle_slave_status() will receive a phantom report of a
device on #0, but it will not find one.

Signed-off-by: default avatarRichard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: default avatarPierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20220914160248.1047627-6-rf@opensource.cirrus.com


Signed-off-by: default avatarVinod Koul <vkoul@kernel.org>
parent 0c5e99c4

drivers/soundwire/bus.c

+21 −8
Original line number Diff line number Diff line
@@ -729,7 +729,7 @@ void sdw_extract_slave_id(struct sdw_bus *bus, 
}

EXPORT_SYMBOL(sdw_extract_slave_id);



static int sdw_program_device_num(struct sdw_bus *bus)

static int sdw_program_device_num(struct sdw_bus *bus, bool *programmed)

{

	u8 buf[SDW_NUM_DEV_ID_REGISTERS] = {0};

	struct sdw_slave *slave, *_s;
@@ -739,6 +739,8 @@ static int sdw_program_device_num(struct sdw_bus *bus) 
	int count = 0, ret;

	u64 addr;



	*programmed = false;



	/* No Slave, so use raw xfer api */

	ret = sdw_fill_msg(&msg, NULL, SDW_SCP_DEVID_0,

			   SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf);
@@ -797,6 +799,8 @@ static int sdw_program_device_num(struct sdw_bus *bus) 
					return ret;

				}



				*programmed = true;



				break;

			}

		}
@@ -1756,7 +1760,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus, 
{

	enum sdw_slave_status prev_status;

	struct sdw_slave *slave;

	bool attached_initializing;

	bool attached_initializing, id_programmed;

	int i, ret = 0;



	/* first check if any Slaves fell off the bus */
@@ -1787,14 +1791,23 @@ int sdw_handle_slave_status(struct sdw_bus *bus, 


	if (status[0] == SDW_SLAVE_ATTACHED) {

		dev_dbg(bus->dev, "Slave attached, programming device number\n");

		ret = sdw_program_device_num(bus);

		if (ret < 0)

			dev_err(bus->dev, "Slave attach failed: %d\n", ret);



		/*

		 * programming a device number will have side effects,

		 * so we deal with other devices at a later time

		 * Programming a device number will have side effects,

		 * so we deal with other devices at a later time.

		 * This relies on those devices reporting ATTACHED, which will

		 * trigger another call to this function. This will only

		 * happen if at least one device ID was programmed.

		 * Error returns from sdw_program_device_num() are currently

		 * ignored because there's no useful recovery that can be done.

		 * Returning the error here could result in the current status

		 * of other devices not being handled, because if no device IDs

		 * were programmed there's nothing to guarantee a status change

		 * to trigger another call to this function.

		 */

		return ret;

		sdw_program_device_num(bus, &id_programmed);

		if (id_programmed)

			return 0;

	}



	/* Continue to check other slave statuses */