On 9/7/22 12:14, Richard Fitzgerald wrote:

When the bus manager is removed sdw_bus_master_delete() should not be deleting the struct sdw_slave objects until the bus manager has been stopped. The first step of removing child drivers should only be calling device_unregister() on the child. The counterpart to sdw_drv_probe() is sdw_drv_remove(), not sdw_delete_slave().

The sdw_slave objects are created by the bus manager probe() from ACPI/DT information. They are not created when a child driver probes so should not be deleted by a child driver remove.

Change-Id: I25cc145df12fdc7c126f8f594a5f76eedce25488

spurious Change-Id

Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com

drivers/soundwire/bus.c | 30 ++++++++++++++++++++++++++---- drivers/soundwire/slave.c | 21 +++++++++++++++++---- 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 1327a312be86..5533eb589286 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -146,9 +146,8 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, } EXPORT_SYMBOL(sdw_bus_master_add);

-static int sdw_delete_slave(struct device *dev, void *data) +static int sdw_delete_slave(struct sdw_slave *slave) {

• struct sdw_slave *slave = dev_to_sdw_dev(dev); struct sdw_bus *bus = slave->bus;

  sdw_slave_debugfs_exit(slave);

@@ -163,7 +162,24 @@ static int sdw_delete_slave(struct device *dev, void *data) list_del_init(&slave->node); mutex_unlock(&bus->bus_lock);

• mutex_destroy(&slave->sdw_dev_lock);
• kfree(slave);
• return 0;

+}

+static int sdw_remove_child(struct device *dev, void *data) +{

• /*

• * Do not remove the struct sdw_slave yet. This is created by
  

• * the bus manager probe() from ACPI information and used by the
  

• * bus manager to hold status of each peripheral. Its lifetime
  

• * is that of the bus manager.
  

• */
  

• /* This will call sdw_drv_remove() */ device_unregister(dev);
• return 0;

}

@@ -171,16 +187,22 @@ static int sdw_delete_slave(struct device *dev, void *data)

• sdw_bus_master_delete() - delete the bus master instance
• @bus: bus to be deleted

• • Remove the instance, delete the child devices.

• • Remove the child devices, remove the master instance.
  */

void sdw_bus_master_delete(struct sdw_bus *bus) {

• device_for_each_child(bus->dev, NULL, sdw_delete_slave);

• struct sdw_slave *slave, *tmp;

• device_for_each_child(bus->dev, NULL, sdw_remove_child);

  /* Children have been removed so it is now safe for the bus to stop */ if (bus->ops->remove) bus->ops->remove(bus);

• /* Now the bus is stopped it is safe to free things */

• list_for_each_entry_safe(slave, tmp, &bus->slaves, node)

• sdw_delete_slave(slave);
  

• sdw_master_device_del(bus);

  sdw_bus_debugfs_exit(bus);

diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index c1c1a2ac293a..b6161d002b97 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -10,10 +10,23 @@

static void sdw_slave_release(struct device *dev) {

• struct sdw_slave *slave = dev_to_sdw_dev(dev);
• mutex_destroy(&slave->sdw_dev_lock);
• kfree(slave);

• /*

• * The release() callback should not be empty
  

• * (see Documentation/core-api/kobject.rst) but the ownership
  

• * of struct sdw_slave is muddled. It is used for two separate
  

• * purposes:
  

• * 1) by the bus driver to track its own state information for
  

• *    physical devices on the bus and found in ACPI/DT, whether
  

• *    or not there is a child driver for it;
  

• * 2) to hold the child driver object.
  

• *
  

• * The struct sdw_slave cannot be freed when the child driver
  

• * is released because it is holding info used by the bus
  

• * driver. It is freed when the bus driver is removed.
  

• *
  

• * Until the ownership issue is untangled this cannot free
  

• * the struct sdw_slave object containing the child dev.
  

• */
  

}

struct device_type sdw_slave_type = {