ASoC: rt711-sdca: enable pm_runtime in probe, keep status as 'suspended'

In stress cases involving module insertion/removal followed by
playback/capture, it can happen that capture/playback is started
before the codec enumeration completes.

The codec driver registers its components with the ASoC framework
during the probe stage, so there is currently no way for the card
creation to wait for the codec enumeration/initialization to complete.

In addition, when the capture/playback starts, the ASoC framework uses
pm_runtime_get_sync() to properly refcount and power-manage
devices. This is problematic in the SoundWire case because pm_runtime
is enabled during the enumeration/initialization stage, so
pm_runtime_get_sync() will return -EACCESS which is
ignored. Additional errors will happen when setting the pm_runtime
status as 'active' because the parent is not properly resumed,
resulting in an error such as:

"rt711 sdw:0:025d:0711:00: runtime PM trying to activate child device
sdw:0:025d:0711:00 but parent (sdw-master-0) is not active"

This patch suggests enabling pm_runtime during the probe, but marking
the device as 'active' only after it is enumerated. That will force a
dependency between the card and the codec, pm_runtime_get_sync() will
have to wait for the codec device to resume and hence implicitly wait
for the enumeration/initialization to be completed. In the nominal
case where the codec device is already active the get_sync() would
only perform a ref-count increase.

Closes: https://github.com/thesofproject/linux/issues/4328
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20230802153629.53576-6-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/sound/soc/codecs/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c
index 23f23f714b3991afff4fb96dc3ce336b5678558a..935e597022d3242187b378107e302a36bedd17f5 100644 (file)
--- a/sound/soc/codecs/rt711-sdca-sdw.c
+++ b/sound/soc/codecs/rt711-sdca-sdw.c
@@ -366,8 +366,7 @@ static int rt711_sdca_sdw_remove(struct sdw_slave *slave)
                cancel_delayed_work_sync(&rt711->jack_btn_check_work);
        }
 
-       if (rt711->first_hw_init)
-               pm_runtime_disable(&slave->dev);
+       pm_runtime_disable(&slave->dev);
 
        mutex_destroy(&rt711->calibrate_mutex);
        mutex_destroy(&rt711->disable_irq_lock);

diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c
index bd0f5e05874bb244ec2139682a4d8528ffce0dce..447154cb60104d31bb66ef268d7b60c513e273e3 100644 (file)
--- a/sound/soc/codecs/rt711-sdca.c
+++ b/sound/soc/codecs/rt711-sdca.c
@@ -507,6 +507,10 @@ static int rt711_sdca_set_jack_detect(struct snd_soc_component *component,
 
        rt711->hs_jack = hs_jack;
 
+       /* we can only resume if the device was initialized at least once */
+       if (!rt711->first_hw_init)
+               return 0;
+
        ret = pm_runtime_resume_and_get(component->dev);
        if (ret < 0) {
                if (ret != -EACCES) {
@@ -1215,6 +1219,9 @@ static int rt711_sdca_probe(struct snd_soc_component *component)
        rt711_sdca_parse_dt(rt711, &rt711->slave->dev);
        rt711->component = component;
 
+       if (!rt711->first_hw_init)
+               return 0;
+
        ret = pm_runtime_resume(component->dev);
        if (ret < 0 && ret != -EACCES)
                return ret;
@@ -1434,9 +1441,27 @@ int rt711_sdca_init(struct device *dev, struct regmap *regmap,
                        rt711_sdca_dai,
                        ARRAY_SIZE(rt711_sdca_dai));
 
-       dev_dbg(&slave->dev, "%s\n", __func__);
+       if (ret < 0)
+               return ret;
 
-       return ret;
+       /* set autosuspend parameters */
+       pm_runtime_set_autosuspend_delay(dev, 3000);
+       pm_runtime_use_autosuspend(dev);
+
+       /* make sure the device does not suspend immediately */
+       pm_runtime_mark_last_busy(dev);
+
+       pm_runtime_enable(dev);
+
+       /* important note: the device is NOT tagged as 'active' and will remain
+        * 'suspended' until the hardware is enumerated/initialized. This is required
+        * to make sure the ASoC framework use of pm_runtime_get_sync() does not silently
+        * fail with -EACCESS because of race conditions between card creation and enumeration
+        */
+
+       dev_dbg(dev, "%s\n", __func__);
+
+       return 0;
 }
 
 static void rt711_sdca_vd0_io_init(struct rt711_sdca_priv *rt711)
@@ -1511,20 +1536,11 @@ int rt711_sdca_io_init(struct device *dev, struct sdw_slave *slave)
                regcache_cache_bypass(rt711->mbq_regmap, true);
        } else {
                /*
-                * PM runtime is only enabled when a Slave reports as Attached
+                * PM runtime status is marked as 'active' only when a Slave reports as Attached
                 */
 
-               /* set autosuspend parameters */
-               pm_runtime_set_autosuspend_delay(&slave->dev, 3000);
-               pm_runtime_use_autosuspend(&slave->dev);
-
                /* update count of parent 'active' children */
                pm_runtime_set_active(&slave->dev);
-
-               /* make sure the device does not suspend immediately */
-               pm_runtime_mark_last_busy(&slave->dev);
-
-               pm_runtime_enable(&slave->dev);
        }
 
        pm_runtime_get_noresume(&slave->dev);