[PATCH v2 2/4] ASoC: SDCA: Fix cleanup inversion in class driver

[Charles Keepax <ckeepax@opensource.cirrus.com>]

From: Richard Fitzgerald <rf@opensource.cirrus.com>

Fix inverted cleanup of the SoundWire IRQ and the function drivers
that use it.

The devm cleanup function to call sdca_dev_unregister_functions() was
being registered at the end of class_sdw_probe(). The bus core
creates the parent SoundWire IRQ handler after class_sdw_probe() has
returned, and it registers a devm cleanup handler at the same time.

This led to a cleanup inversion where the devm cleanup for the parent
Soundwire IRQ runs before the handler that removes the function drivers.
So the parent IRQ is destroyed before the function drivers had a chance
to do any cleanup and remove their IRQ handlers.

Move the registrations of the function driver cleanup into
class_boot_work() after the function drivers are registered, so that it
runs before the cleanup of the parent SoundWire IRQ handler.

Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---

Changes since v1:
 - Move cancel_work_sync() on boot_work to bus remove.

 sound/soc/sdca/sdca_class.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c
index 7af4e5d1b347f..6e9b66f718019 100644
--- a/sound/soc/sdca/sdca_class.c
+++ b/sound/soc/sdca/sdca_class.c
@@ -137,6 +137,13 @@ static const struct regmap_config class_dev_regmap_config = {
 	.unlock			= class_regmap_unlock,
 };
 
+static void class_remove_functions(void *data)
+{
+	struct sdca_class_drv *drv = data;
+
+	sdca_dev_unregister_functions(drv->sdw);
+}
+
 static void class_boot_work(struct work_struct *work)
 {
 	struct sdca_class_drv *drv = container_of(work,
@@ -157,6 +164,11 @@ static void class_boot_work(struct work_struct *work)
 	if (ret)
 		goto err;
 
+	/* Ensure function drivers are removed before the IRQ is destroyed */
+	ret = devm_add_action_or_reset(drv->dev, class_remove_functions, drv);
+	if (ret)
+		goto err;
+
 	dev_dbg(drv->dev, "boot work complete\n");
 
 	pm_runtime_mark_last_busy(drv->dev);
@@ -168,15 +180,6 @@ static void class_boot_work(struct work_struct *work)
 	pm_runtime_put_sync(drv->dev);
 }
 
-static void class_dev_remove(void *data)
-{
-	struct sdca_class_drv *drv = data;
-
-	cancel_work_sync(&drv->boot_work);
-
-	sdca_dev_unregister_functions(drv->sdw);
-}
-
 static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id)
 {
 	struct device *dev = &sdw->dev;
@@ -230,15 +233,19 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id
 	if (ret)
 		return ret;
 
-	ret = devm_add_action_or_reset(dev, class_dev_remove, drv);
-	if (ret)
-		return ret;
-
 	queue_work(system_long_wq, &drv->boot_work);
 
 	return 0;
 }
 
+static void class_sdw_remove(struct sdw_slave *sdw)
+{
+	struct device *dev = &sdw->dev;
+	struct sdca_class_drv *drv = dev_get_drvdata(dev);
+
+	cancel_work_sync(&drv->boot_work);
+}
+
 static int class_suspend(struct device *dev)
 {
 	struct sdca_class_drv *drv = dev_get_drvdata(dev);
@@ -330,6 +337,7 @@ static struct sdw_driver class_sdw_driver = {
 	},
 
 	.probe		= class_sdw_probe,
+	.remove		= class_sdw_remove,
 	.id_table	= class_sdw_id,
 	.ops		= &class_sdw_ops,
 };
-- 
2.47.3