[PATCH 00/16] ASoC: SoundWire codecs: improve pm_runtime handling
This patchset improves the pm_runtime behavior in rare corner cases identified by the Intel CI in the last 6 months.
a) in stress-tests, it's not uncommon to see the following type of warnings when the codec reports as ATTACHED
"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 warning was not correlated with any functional issue, but it exposed a design issue on when to enable pm_runtime. The recommended practice in the pm_runtime documentation is to keep the devices in 'suspended' mode and mark them as 'active' when they are really functional.
b) enabling pm_runtime when the codec reports as ATTACHED also creates a problematic case when the ASoC pm_runtime_get_sync() will silently fail due to the -EACCESS error handling. This can happen when playback starts before the codec is enumerated.
This patchset modifies the initial stages so that codecs are pm_runtime enabled in the .probe() callback, but become pm_runtime 'active' only when they report present. This is better aligned with the design of the pm_runtime helpers and improved CI results significantly.
This patchset modifies all existing SoundWire codecs (except Qualcomm ones), but the pattern of changes is exactly the same in all patches.
Pierre-Louis Bossart (16): ASoC: SoundWire codecs: return error status in probe ASoC: SoundWire codecs: make regmap cache-only in probe ASoC: rt5682-sdw: make regmap cache-only in probe ASoC: rt711: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt711-sdca: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt700: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt1712-sdca: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt712-sdca-dmic: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt715: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt715-sdca: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt1308-sdw: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt1316-sdw: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt1318-sdw: enable pm_runtime in probe, keep status as 'suspended' ASoC: rt5682-sdw: enable pm_runtime in probe, keep status as 'suspended' ASoC: max98363: enable pm_runtime in probe, keep status as 'suspended' ASoC: max98373-sdw: enable pm_runtime in probe, keep status as 'suspended'
sound/soc/codecs/max98363.c | 41 +++++++++++++--------- sound/soc/codecs/max98373-sdw.c | 47 ++++++++++++++----------- sound/soc/codecs/rt1308-sdw.c | 54 +++++++++++++++------------- sound/soc/codecs/rt1316-sdw.c | 44 ++++++++++++++--------- sound/soc/codecs/rt1318-sdw.c | 43 ++++++++++++++--------- sound/soc/codecs/rt5682-sdw.c | 56 ++++++++++++++++++------------ sound/soc/codecs/rt5682.c | 3 ++ sound/soc/codecs/rt700-sdw.c | 7 ++-- sound/soc/codecs/rt700.c | 46 +++++++++++++++--------- sound/soc/codecs/rt711-sdca-sdw.c | 3 +- sound/soc/codecs/rt711-sdca.c | 48 +++++++++++++++++-------- sound/soc/codecs/rt711-sdw.c | 7 ++-- sound/soc/codecs/rt711.c | 47 ++++++++++++++++--------- sound/soc/codecs/rt712-sdca-dmic.c | 47 +++++++++++++++---------- sound/soc/codecs/rt712-sdca-sdw.c | 3 +- sound/soc/codecs/rt712-sdca.c | 45 ++++++++++++++++-------- sound/soc/codecs/rt715-sdca-sdw.c | 5 +-- sound/soc/codecs/rt715-sdca.c | 41 ++++++++++++++++------ sound/soc/codecs/rt715-sdw.c | 9 ++--- sound/soc/codecs/rt715.c | 41 +++++++++++++++------- 20 files changed, 392 insertions(+), 245 deletions(-)
For some reason the first batch of SoundWire codec drivers squelch errors in the SoundWire probe callback.
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 --- sound/soc/codecs/rt1308-sdw.c | 4 +--- sound/soc/codecs/rt5682-sdw.c | 4 +--- sound/soc/codecs/rt700-sdw.c | 4 +--- sound/soc/codecs/rt711-sdw.c | 4 +--- sound/soc/codecs/rt715-sdw.c | 4 +--- 5 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index f43520ca3187..a7740549d35c 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -715,9 +715,7 @@ static int rt1308_sdw_probe(struct sdw_slave *slave, if (IS_ERR(regmap)) return PTR_ERR(regmap);
- rt1308_sdw_init(&slave->dev, regmap, slave); - - return 0; + return rt1308_sdw_init(&slave->dev, regmap, slave); }
static int rt1308_sdw_remove(struct sdw_slave *slave) diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 4968a8c0064d..7d53dd62ce17 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -674,9 +674,7 @@ static int rt5682_sdw_probe(struct sdw_slave *slave, if (IS_ERR(regmap)) return -EINVAL;
- rt5682_sdw_init(&slave->dev, regmap, slave); - - return 0; + return rt5682_sdw_init(&slave->dev, regmap, slave); }
static int rt5682_sdw_remove(struct sdw_slave *slave) diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index 8b28e47775cc..53e7973e0bf9 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -452,9 +452,7 @@ static int rt700_sdw_probe(struct sdw_slave *slave, if (IS_ERR(regmap)) return PTR_ERR(regmap);
- rt700_init(&slave->dev, sdw_regmap, regmap, slave); - - return 0; + return rt700_init(&slave->dev, sdw_regmap, regmap, slave); }
static int rt700_sdw_remove(struct sdw_slave *slave) diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index 33dced388f9e..530d1ae32c04 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -453,9 +453,7 @@ static int rt711_sdw_probe(struct sdw_slave *slave, if (IS_ERR(regmap)) return PTR_ERR(regmap);
- rt711_init(&slave->dev, sdw_regmap, regmap, slave); - - return 0; + return rt711_init(&slave->dev, sdw_regmap, regmap, slave); }
static int rt711_sdw_remove(struct sdw_slave *slave) diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c index 6db87442b783..d09b3061096d 100644 --- a/sound/soc/codecs/rt715-sdw.c +++ b/sound/soc/codecs/rt715-sdw.c @@ -508,9 +508,7 @@ static int rt715_sdw_probe(struct sdw_slave *slave, if (IS_ERR(regmap)) return PTR_ERR(regmap);
- rt715_init(&slave->dev, sdw_regmap, regmap, slave); - - return 0; + return rt715_init(&slave->dev, sdw_regmap, regmap, slave); }
static int rt715_sdw_remove(struct sdw_slave *slave)
The SoundWire bus may start after the probe where the SoundWire ASoC components are registered. This creates a time window where the card can be created and the registers be accessed.
As discussed on the mailing list, we can't really control when codecs are enumerated and initialized, but we can make sure the access to the codecs is cached until the hardware is accessible.
This patch configures regcache_cache_only() with a 'true' parameter in the probe function, and a 'false' parameter in the io_init routine.
The rt5682 is handled through a different patch due to its specific cache handling.
Link: https://lore.kernel.org/alsa-devel/20230503144102.242240-1-krzysztof.kozlows... 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 --- sound/soc/codecs/max98363.c | 7 ++++--- sound/soc/codecs/max98373-sdw.c | 7 ++++--- sound/soc/codecs/rt1308-sdw.c | 7 ++++--- sound/soc/codecs/rt1316-sdw.c | 4 +++- sound/soc/codecs/rt1318-sdw.c | 4 +++- sound/soc/codecs/rt700.c | 7 ++++--- sound/soc/codecs/rt711-sdca.c | 8 ++++++-- sound/soc/codecs/rt711.c | 7 ++++--- sound/soc/codecs/rt712-sdca-dmic.c | 7 +++++-- sound/soc/codecs/rt712-sdca.c | 7 +++++-- sound/soc/codecs/rt715-sdca.c | 7 +++++++ sound/soc/codecs/rt715.c | 4 ++++ 12 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/sound/soc/codecs/max98363.c b/sound/soc/codecs/max98363.c index b5c69bba0e48..80a1cb482183 100644 --- a/sound/soc/codecs/max98363.c +++ b/sound/soc/codecs/max98363.c @@ -160,10 +160,9 @@ static int max98363_io_init(struct sdw_slave *slave) struct max98363_priv *max98363 = dev_get_drvdata(dev); int ret, reg;
- if (max98363->first_hw_init) { - regcache_cache_only(max98363->regmap, false); + regcache_cache_only(max98363->regmap, false); + if (max98363->first_hw_init) regcache_cache_bypass(max98363->regmap, true); - }
/* * PM runtime is only enabled when a Slave reports as Attached @@ -409,6 +408,8 @@ static int max98363_init(struct sdw_slave *slave, struct regmap *regmap) max98363->regmap = regmap; max98363->slave = slave;
+ regcache_cache_only(max98363->regmap, true); + max98363->hw_init = false; max98363->first_hw_init = false;
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c index df92242af960..92d2b872f9f8 100644 --- a/sound/soc/codecs/max98373-sdw.c +++ b/sound/soc/codecs/max98373-sdw.c @@ -361,10 +361,9 @@ static int max98373_io_init(struct sdw_slave *slave) struct device *dev = &slave->dev; struct max98373_priv *max98373 = dev_get_drvdata(dev);
- if (max98373->first_hw_init) { - regcache_cache_only(max98373->regmap, false); + regcache_cache_only(max98373->regmap, false); + if (max98373->first_hw_init) regcache_cache_bypass(max98373->regmap, true); - }
/* * PM runtime is only enabled when a Slave reports as Attached @@ -753,6 +752,8 @@ static int max98373_init(struct sdw_slave *slave, struct regmap *regmap) max98373->regmap = regmap; max98373->slave = slave;
+ regcache_cache_only(max98373->regmap, true); + max98373->cache_num = ARRAY_SIZE(max98373_sdw_cache_reg); max98373->cache = devm_kcalloc(dev, max98373->cache_num, sizeof(*max98373->cache), diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index a7740549d35c..2c4e5330c2df 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -218,10 +218,9 @@ static int rt1308_io_init(struct device *dev, struct sdw_slave *slave) if (rt1308->hw_init) return 0;
- if (rt1308->first_hw_init) { - regcache_cache_only(rt1308->regmap, false); + regcache_cache_only(rt1308->regmap, false); + if (rt1308->first_hw_init) regcache_cache_bypass(rt1308->regmap, true); - }
/* * PM runtime is only enabled when a Slave reports as Attached @@ -688,6 +687,8 @@ static int rt1308_sdw_init(struct device *dev, struct regmap *regmap, rt1308->sdw_slave = slave; rt1308->regmap = regmap;
+ regcache_cache_only(rt1308->regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c index 10a53c8d4874..57abbe2de7cf 100644 --- a/sound/soc/codecs/rt1316-sdw.c +++ b/sound/soc/codecs/rt1316-sdw.c @@ -272,8 +272,8 @@ static int rt1316_io_init(struct device *dev, struct sdw_slave *slave) if (rt1316->hw_init) return 0;
+ regcache_cache_only(rt1316->regmap, false); if (rt1316->first_hw_init) { - regcache_cache_only(rt1316->regmap, false); regcache_cache_bypass(rt1316->regmap, true); } else { /* @@ -674,6 +674,8 @@ static int rt1316_sdw_init(struct device *dev, struct regmap *regmap, rt1316->sdw_slave = slave; rt1316->regmap = regmap;
+ regcache_cache_only(rt1316->regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present diff --git a/sound/soc/codecs/rt1318-sdw.c b/sound/soc/codecs/rt1318-sdw.c index 16d750102c8c..d7803342f5c2 100644 --- a/sound/soc/codecs/rt1318-sdw.c +++ b/sound/soc/codecs/rt1318-sdw.c @@ -408,8 +408,8 @@ static int rt1318_io_init(struct device *dev, struct sdw_slave *slave) if (rt1318->hw_init) return 0;
+ regcache_cache_only(rt1318->regmap, false); if (rt1318->first_hw_init) { - regcache_cache_only(rt1318->regmap, false); regcache_cache_bypass(rt1318->regmap, true); } else { /* @@ -752,6 +752,8 @@ static int rt1318_sdw_init(struct device *dev, struct regmap *regmap, rt1318->sdw_slave = slave; rt1318->regmap = regmap;
+ regcache_cache_only(rt1318->regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present diff --git a/sound/soc/codecs/rt700.c b/sound/soc/codecs/rt700.c index a04b9246256b..b774349dfdae 100644 --- a/sound/soc/codecs/rt700.c +++ b/sound/soc/codecs/rt700.c @@ -1099,6 +1099,8 @@ int rt700_init(struct device *dev, struct regmap *sdw_regmap, rt700->sdw_regmap = sdw_regmap; rt700->regmap = regmap;
+ regcache_cache_only(rt700->regmap, true); + mutex_init(&rt700->disable_irq_lock);
INIT_DELAYED_WORK(&rt700->jack_detect_work, @@ -1132,10 +1134,9 @@ int rt700_io_init(struct device *dev, struct sdw_slave *slave) if (rt700->hw_init) return 0;
- if (rt700->first_hw_init) { - regcache_cache_only(rt700->regmap, false); + regcache_cache_only(rt700->regmap, false); + if (rt700->first_hw_init) regcache_cache_bypass(rt700->regmap, true); - }
/* * PM runtime is only enabled when a Slave reports as Attached diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c index 07640d2f6e56..bd0f5e05874b 100644 --- a/sound/soc/codecs/rt711-sdca.c +++ b/sound/soc/codecs/rt711-sdca.c @@ -1406,6 +1406,9 @@ int rt711_sdca_init(struct device *dev, struct regmap *regmap, rt711->regmap = regmap; rt711->mbq_regmap = mbq_regmap;
+ regcache_cache_only(rt711->regmap, true); + regcache_cache_only(rt711->mbq_regmap, true); + mutex_init(&rt711->calibrate_mutex); mutex_init(&rt711->disable_irq_lock);
@@ -1500,10 +1503,11 @@ int rt711_sdca_io_init(struct device *dev, struct sdw_slave *slave) if (rt711->hw_init) return 0;
+ regcache_cache_only(rt711->regmap, false); + regcache_cache_only(rt711->mbq_regmap, false); + if (rt711->first_hw_init) { - regcache_cache_only(rt711->regmap, false); regcache_cache_bypass(rt711->regmap, true); - regcache_cache_only(rt711->mbq_regmap, false); regcache_cache_bypass(rt711->mbq_regmap, true); } else { /* diff --git a/sound/soc/codecs/rt711.c b/sound/soc/codecs/rt711.c index af53cbcc7bf2..0ca955e2f4e7 100644 --- a/sound/soc/codecs/rt711.c +++ b/sound/soc/codecs/rt711.c @@ -1183,6 +1183,8 @@ int rt711_init(struct device *dev, struct regmap *sdw_regmap, rt711->sdw_regmap = sdw_regmap; rt711->regmap = regmap;
+ regcache_cache_only(rt711->regmap, true); + mutex_init(&rt711->calibrate_mutex); mutex_init(&rt711->disable_irq_lock);
@@ -1219,10 +1221,9 @@ int rt711_io_init(struct device *dev, struct sdw_slave *slave) if (rt711->hw_init) return 0;
- if (rt711->first_hw_init) { - regcache_cache_only(rt711->regmap, false); + regcache_cache_only(rt711->regmap, false); + if (rt711->first_hw_init) regcache_cache_bypass(rt711->regmap, true); - }
/* * PM runtime is only enabled when a Slave reports as Attached diff --git a/sound/soc/codecs/rt712-sdca-dmic.c b/sound/soc/codecs/rt712-sdca-dmic.c index 869cc7bfd178..0102bad0b66a 100644 --- a/sound/soc/codecs/rt712-sdca-dmic.c +++ b/sound/soc/codecs/rt712-sdca-dmic.c @@ -182,10 +182,10 @@ static int rt712_sdca_dmic_io_init(struct device *dev, struct sdw_slave *slave) if (rt712->hw_init) return 0;
+ regcache_cache_only(rt712->regmap, false); + regcache_cache_only(rt712->mbq_regmap, false); if (rt712->first_hw_init) { - regcache_cache_only(rt712->regmap, false); regcache_cache_bypass(rt712->regmap, true); - regcache_cache_only(rt712->mbq_regmap, false); regcache_cache_bypass(rt712->mbq_regmap, true); } else { /* @@ -777,6 +777,9 @@ static int rt712_sdca_dmic_init(struct device *dev, struct regmap *regmap, rt712->regmap = regmap; rt712->mbq_regmap = mbq_regmap;
+ regcache_cache_only(rt712->regmap, true); + regcache_cache_only(rt712->mbq_regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present diff --git a/sound/soc/codecs/rt712-sdca.c b/sound/soc/codecs/rt712-sdca.c index 89d245655ca4..88f6c895722e 100644 --- a/sound/soc/codecs/rt712-sdca.c +++ b/sound/soc/codecs/rt712-sdca.c @@ -1183,6 +1183,9 @@ int rt712_sdca_init(struct device *dev, struct regmap *regmap, rt712->regmap = regmap; rt712->mbq_regmap = mbq_regmap;
+ regcache_cache_only(rt712->regmap, true); + regcache_cache_only(rt712->mbq_regmap, true); + mutex_init(&rt712->calibrate_mutex); mutex_init(&rt712->disable_irq_lock);
@@ -1224,10 +1227,10 @@ int rt712_sdca_io_init(struct device *dev, struct sdw_slave *slave) if (rt712->hw_init) return 0;
+ regcache_cache_only(rt712->regmap, false); + regcache_cache_only(rt712->mbq_regmap, false); if (rt712->first_hw_init) { - regcache_cache_only(rt712->regmap, false); regcache_cache_bypass(rt712->regmap, true); - regcache_cache_only(rt712->mbq_regmap, false); regcache_cache_bypass(rt712->mbq_regmap, true); } else { /* diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c index b989f907784b..176340a43446 100644 --- a/sound/soc/codecs/rt715-sdca.c +++ b/sound/soc/codecs/rt715-sdca.c @@ -977,6 +977,10 @@ int rt715_sdca_init(struct device *dev, struct regmap *mbq_regmap, rt715->regmap = regmap; rt715->mbq_regmap = mbq_regmap; rt715->hw_sdw_ver = slave->id.sdw_version; + + regcache_cache_only(rt715->regmap, true); + regcache_cache_only(rt715->mbq_regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present @@ -1000,6 +1004,9 @@ int rt715_sdca_io_init(struct device *dev, struct sdw_slave *slave) if (rt715->hw_init) return 0;
+ regcache_cache_only(rt715->regmap, false); + regcache_cache_only(rt715->mbq_regmap, false); + /* * PM runtime is only enabled when a Slave reports as Attached */ diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index 6c2e165dd621..1bd2fe8aa625 100644 --- a/sound/soc/codecs/rt715.c +++ b/sound/soc/codecs/rt715.c @@ -984,6 +984,8 @@ int rt715_init(struct device *dev, struct regmap *sdw_regmap, rt715->regmap = regmap; rt715->sdw_regmap = sdw_regmap;
+ regcache_cache_only(rt715->regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present @@ -1006,6 +1008,8 @@ int rt715_io_init(struct device *dev, struct sdw_slave *slave) if (rt715->hw_init) return 0;
+ regcache_cache_only(rt715->regmap, false); + /* * PM runtime is only enabled when a Slave reports as Attached */
The RT5682 needs specific attention: there are two regmap in rt5682_priv struct, one is sdw_regmap which is for IO transfer, and the other is used for registers control.
We need to set both regmaps when we set cache only. Because if we set rt5682->sdw_regmap only, rt5682->regmap won't set/get the right value when it call regmap_write/read(rt5682->sdw_regmap, ...). If we set rt5682->regmap only, regmap_write(rt5682->sdw_regmap, ...) is used in rt5682_clock_config which will be called by the ..bus_config ops.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/codecs/rt5682-sdw.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 7d53dd62ce17..6726458cf329 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -322,6 +322,9 @@ static int rt5682_sdw_init(struct device *dev, struct regmap *regmap, return ret; }
+ regcache_cache_only(rt5682->sdw_regmap, true); + regcache_cache_only(rt5682->regmap, true); + /* * Mark hw_init to false * HW init will be performed when device reports present @@ -352,6 +355,11 @@ static int rt5682_io_init(struct device *dev, struct sdw_slave *slave) if (rt5682->hw_init) return 0;
+ regcache_cache_only(rt5682->sdw_regmap, false); + regcache_cache_only(rt5682->regmap, false); + if (rt5682->first_hw_init) + regcache_cache_bypass(rt5682->regmap, true); + /* * PM runtime is only enabled when a Slave reports as Attached */ @@ -371,11 +379,6 @@ static int rt5682_io_init(struct device *dev, struct sdw_slave *slave)
pm_runtime_get_noresume(&slave->dev);
- if (rt5682->first_hw_init) { - regcache_cache_only(rt5682->regmap, false); - regcache_cache_bypass(rt5682->regmap, true); - } - while (loop > 0) { regmap_read(rt5682->regmap, RT5682_DEVICE_ID, &val); if (val == DEVICE_ID) @@ -705,6 +708,7 @@ static int __maybe_unused rt5682_dev_suspend(struct device *dev)
cancel_delayed_work_sync(&rt5682->jack_detect_work);
+ regcache_cache_only(rt5682->sdw_regmap, true); regcache_cache_only(rt5682->regmap, true); regcache_mark_dirty(rt5682->regmap);
@@ -769,6 +773,7 @@ static int __maybe_unused rt5682_dev_resume(struct device *dev)
regmap_sync: slave->unattach_request = 0; + regcache_cache_only(rt5682->sdw_regmap, false); regcache_cache_only(rt5682->regmap, false); regcache_sync(rt5682->regmap);
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 --- sound/soc/codecs/rt711-sdw.c | 3 +-- sound/soc/codecs/rt711.c | 40 ++++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index 530d1ae32c04..3f5773310ae8 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -466,8 +466,7 @@ static int rt711_sdw_remove(struct sdw_slave *slave) cancel_work_sync(&rt711->calibration_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.c b/sound/soc/codecs/rt711.c index 0ca955e2f4e7..66eaed13b0d6 100644 --- a/sound/soc/codecs/rt711.c +++ b/sound/soc/codecs/rt711.c @@ -462,6 +462,10 @@ static int rt711_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) { @@ -941,6 +945,9 @@ static int rt711_probe(struct snd_soc_component *component) rt711_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; @@ -1206,8 +1213,25 @@ int rt711_init(struct device *dev, struct regmap *sdw_regmap, &soc_codec_dev_rt711, rt711_dai, ARRAY_SIZE(rt711_dai)); + if (ret < 0) + return ret;
- dev_dbg(&slave->dev, "%s\n", __func__); + /* 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 ret; } @@ -1226,22 +1250,12 @@ int rt711_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt711->regmap, true);
/* - * 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 */ - if (!rt711->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(&slave->dev, 3000); - pm_runtime_use_autosuspend(&slave->dev); - + if (!rt711->first_hw_init) /* 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);
rt711_reset(rt711->regmap);
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 --- sound/soc/codecs/rt711-sdca-sdw.c | 3 +-- sound/soc/codecs/rt711-sdca.c | 40 +++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c index 23f23f714b39..935e597022d3 100644 --- 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 bd0f5e05874b..447154cb6010 100644 --- 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);
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt700-sdw.c | 3 +-- sound/soc/codecs/rt700.c | 39 ++++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index 53e7973e0bf9..52c33d56b143 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -464,8 +464,7 @@ static int rt700_sdw_remove(struct sdw_slave *slave) cancel_delayed_work_sync(&rt700->jack_btn_check_work); }
- if (rt700->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; } diff --git a/sound/soc/codecs/rt700.c b/sound/soc/codecs/rt700.c index b774349dfdae..0ebf344a1b60 100644 --- a/sound/soc/codecs/rt700.c +++ b/sound/soc/codecs/rt700.c @@ -320,6 +320,10 @@ static int rt700_set_jack_detect(struct snd_soc_component *component,
rt700->hs_jack = hs_jack;
+ /* we can only resume if the device was initialized at least once */ + if (!rt700->first_hw_init) + return 0; + ret = pm_runtime_resume_and_get(component->dev); if (ret < 0) { if (ret != -EACCES) { @@ -823,6 +827,9 @@ static int rt700_probe(struct snd_soc_component *component)
rt700->component = component;
+ if (!rt700->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -1119,10 +1126,26 @@ int rt700_init(struct device *dev, struct regmap *sdw_regmap, &soc_codec_dev_rt700, rt700_dai, ARRAY_SIZE(rt700_dai)); + if (ret < 0) + 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(&slave->dev, "%s\n", __func__);
- return ret; + return 0; }
int rt700_io_init(struct device *dev, struct sdw_slave *slave) @@ -1141,20 +1164,10 @@ int rt700_io_init(struct device *dev, struct sdw_slave *slave) /* * PM runtime is only enabled when a Slave reports as Attached */ - if (!rt700->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(&slave->dev, 3000); - pm_runtime_use_autosuspend(&slave->dev); - - /* update count of parent 'active' children */ + if (!rt700->first_hw_init) + /* PM runtime status is marked as 'active' only when a Slave reports as Attached */ 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);
/* reset */
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt712-sdca-sdw.c | 3 +-- sound/soc/codecs/rt712-sdca.c | 38 +++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/rt712-sdca-sdw.c b/sound/soc/codecs/rt712-sdca-sdw.c index 6bc50396a0f6..6b644a89c589 100644 --- a/sound/soc/codecs/rt712-sdca-sdw.c +++ b/sound/soc/codecs/rt712-sdca-sdw.c @@ -363,8 +363,7 @@ static int rt712_sdca_sdw_remove(struct sdw_slave *slave) cancel_delayed_work_sync(&rt712->jack_btn_check_work); }
- if (rt712->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
mutex_destroy(&rt712->calibrate_mutex); mutex_destroy(&rt712->disable_irq_lock); diff --git a/sound/soc/codecs/rt712-sdca.c b/sound/soc/codecs/rt712-sdca.c index 88f6c895722e..7077ff6ba1f4 100644 --- a/sound/soc/codecs/rt712-sdca.c +++ b/sound/soc/codecs/rt712-sdca.c @@ -453,6 +453,9 @@ static int rt712_sdca_set_jack_detect(struct snd_soc_component *component,
rt712->hs_jack = hs_jack;
+ if (!rt712->first_hw_init) + return 0; + ret = pm_runtime_resume_and_get(component->dev); if (ret < 0) { if (ret != -EACCES) { @@ -960,6 +963,9 @@ static int rt712_sdca_probe(struct snd_soc_component *component) rt712_sdca_parse_dt(rt712, &rt712->slave->dev); rt712->component = component;
+ if (!rt712->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -1210,10 +1216,27 @@ int rt712_sdca_init(struct device *dev, struct regmap *regmap, else ret = devm_snd_soc_register_component(dev, &soc_sdca_dev_rt712, rt712_sdca_dai, 1); + if (ret < 0) + return ret;
- dev_dbg(&slave->dev, "%s\n", __func__); + /* set autosuspend parameters */ + pm_runtime_set_autosuspend_delay(dev, 3000); + pm_runtime_use_autosuspend(dev);
- return ret; + /* 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; }
int rt712_sdca_io_init(struct device *dev, struct sdw_slave *slave) @@ -1234,20 +1257,11 @@ int rt712_sdca_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt712->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);
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt712-sdca-dmic.c | 40 ++++++++++++++++++------------ 1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/rt712-sdca-dmic.c b/sound/soc/codecs/rt712-sdca-dmic.c index 0102bad0b66a..ba08d03e717c 100644 --- a/sound/soc/codecs/rt712-sdca-dmic.c +++ b/sound/soc/codecs/rt712-sdca-dmic.c @@ -189,20 +189,11 @@ static int rt712_sdca_dmic_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt712->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); @@ -608,6 +599,9 @@ static int rt712_sdca_dmic_probe(struct snd_soc_component *component)
rt712->component = component;
+ if (!rt712->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -794,10 +788,27 @@ static int rt712_sdca_dmic_init(struct device *dev, struct regmap *regmap, &soc_sdca_dev_rt712_dmic, rt712_sdca_dmic_dai, ARRAY_SIZE(rt712_sdca_dmic_dai)); + if (ret < 0) + return ret;
- dev_dbg(&slave->dev, "%s\n", __func__); + /* set autosuspend parameters */ + pm_runtime_set_autosuspend_delay(dev, 3000); + pm_runtime_use_autosuspend(dev);
- return ret; + /* 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; }
@@ -957,10 +968,7 @@ static int rt712_sdca_dmic_sdw_probe(struct sdw_slave *slave,
static int rt712_sdca_dmic_sdw_remove(struct sdw_slave *slave) { - struct rt712_sdca_dmic_priv *rt712 = dev_get_drvdata(&slave->dev); - - if (rt712->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; }
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt715-sdw.c | 5 +---- sound/soc/codecs/rt715.c | 37 +++++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c index d09b3061096d..21f37babd148 100644 --- a/sound/soc/codecs/rt715-sdw.c +++ b/sound/soc/codecs/rt715-sdw.c @@ -513,10 +513,7 @@ static int rt715_sdw_probe(struct sdw_slave *slave,
static int rt715_sdw_remove(struct sdw_slave *slave) { - struct rt715_priv *rt715 = dev_get_drvdata(&slave->dev); - - if (rt715->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; } diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index 1bd2fe8aa625..79416bb48814 100644 --- a/sound/soc/codecs/rt715.c +++ b/sound/soc/codecs/rt715.c @@ -740,8 +740,12 @@ static int rt715_set_bias_level(struct snd_soc_component *component,
static int rt715_probe(struct snd_soc_component *component) { + struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component); int ret;
+ if (!rt715->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -997,8 +1001,25 @@ int rt715_init(struct device *dev, struct regmap *sdw_regmap, &soc_codec_dev_rt715, rt715_dai, ARRAY_SIZE(rt715_dai)); + 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 + */ + + return 0; }
int rt715_io_init(struct device *dev, struct sdw_slave *slave) @@ -1011,22 +1032,12 @@ int rt715_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_only(rt715->regmap, false);
/* - * 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 */ - if (!rt715->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(&slave->dev, 3000); - pm_runtime_use_autosuspend(&slave->dev); - + if (!rt715->first_hw_init) /* 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);
/* Mute nid=08h/09h */
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt715-sdca-sdw.c | 5 +---- sound/soc/codecs/rt715-sdca.c | 34 ++++++++++++++++++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/rt715-sdca-sdw.c b/sound/soc/codecs/rt715-sdca-sdw.c index df10916bab46..ab54a67a27eb 100644 --- a/sound/soc/codecs/rt715-sdca-sdw.c +++ b/sound/soc/codecs/rt715-sdca-sdw.c @@ -193,10 +193,7 @@ static int rt715_sdca_sdw_probe(struct sdw_slave *slave,
static int rt715_sdca_sdw_remove(struct sdw_slave *slave) { - struct rt715_sdca_priv *rt715 = dev_get_drvdata(&slave->dev); - - if (rt715->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; } diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c index 176340a43446..9fa96fd83d4a 100644 --- a/sound/soc/codecs/rt715-sdca.c +++ b/sound/soc/codecs/rt715-sdca.c @@ -761,8 +761,12 @@ static const struct snd_soc_dapm_route rt715_sdca_audio_map[] = {
static int rt715_sdca_probe(struct snd_soc_component *component) { + struct rt715_sdca_priv *rt715 = snd_soc_component_get_drvdata(component); int ret;
+ if (!rt715->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -992,6 +996,25 @@ int rt715_sdca_init(struct device *dev, struct regmap *mbq_regmap, &soc_codec_dev_rt715_sdca, rt715_sdca_dai, ARRAY_SIZE(rt715_sdca_dai)); + if (ret < 0) + 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 ret; } @@ -1008,21 +1031,12 @@ int rt715_sdca_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_only(rt715->mbq_regmap, false);
/* - * 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 */ if (!rt715->first_hw_init) { - /* 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); - rt715->first_hw_init = true; }
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt1308-sdw.c | 43 ++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index 2c4e5330c2df..c98cd7abef6a 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -223,22 +223,12 @@ static int rt1308_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt1308->regmap, true);
/* - * 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 */ - if (!rt1308->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(&slave->dev, 3000); - pm_runtime_use_autosuspend(&slave->dev); - + if (!rt1308->first_hw_init) /* 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);
/* sw reset */ @@ -625,6 +615,9 @@ static int rt1308_sdw_component_probe(struct snd_soc_component *component) rt1308->component = component; rt1308_sdw_parse_dt(rt1308, &rt1308->sdw_slave->dev);
+ if (!rt1308->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -700,10 +693,27 @@ static int rt1308_sdw_init(struct device *dev, struct regmap *regmap, &soc_component_sdw_rt1308, rt1308_sdw_dai, ARRAY_SIZE(rt1308_sdw_dai)); + if (ret < 0) + return ret;
- dev_dbg(&slave->dev, "%s\n", __func__); + /* set autosuspend parameters */ + pm_runtime_set_autosuspend_delay(dev, 3000); + pm_runtime_use_autosuspend(dev);
- return ret; + /* 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 int rt1308_sdw_probe(struct sdw_slave *slave, @@ -721,10 +731,7 @@ static int rt1308_sdw_probe(struct sdw_slave *slave,
static int rt1308_sdw_remove(struct sdw_slave *slave) { - struct rt1308_sdw_priv *rt1308 = dev_get_drvdata(&slave->dev); - - if (rt1308->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; }
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt1316-sdw.c | 40 +++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c index 57abbe2de7cf..47511f70119a 100644 --- a/sound/soc/codecs/rt1316-sdw.c +++ b/sound/soc/codecs/rt1316-sdw.c @@ -277,20 +277,11 @@ static int rt1316_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt1316->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); @@ -607,6 +598,9 @@ static int rt1316_sdw_component_probe(struct snd_soc_component *component) rt1316->component = component; rt1316_sdw_parse_dt(rt1316, &rt1316->sdw_slave->dev);
+ if (!rt1316->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); if (ret < 0 && ret != -EACCES) return ret; @@ -687,10 +681,27 @@ static int rt1316_sdw_init(struct device *dev, struct regmap *regmap, &soc_component_sdw_rt1316, rt1316_sdw_dai, ARRAY_SIZE(rt1316_sdw_dai)); + if (ret < 0) + return ret;
- dev_dbg(&slave->dev, "%s\n", __func__); + /* set autosuspend parameters */ + pm_runtime_set_autosuspend_delay(dev, 3000); + pm_runtime_use_autosuspend(dev);
- return ret; + /* 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 int rt1316_sdw_probe(struct sdw_slave *slave, @@ -708,10 +719,7 @@ static int rt1316_sdw_probe(struct sdw_slave *slave,
static int rt1316_sdw_remove(struct sdw_slave *slave) { - struct rt1316_sdw_priv *rt1316 = dev_get_drvdata(&slave->dev); - - if (rt1316->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; }
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt1318-sdw.c | 39 +++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/rt1318-sdw.c b/sound/soc/codecs/rt1318-sdw.c index d7803342f5c2..ff364bde4a08 100644 --- a/sound/soc/codecs/rt1318-sdw.c +++ b/sound/soc/codecs/rt1318-sdw.c @@ -413,20 +413,10 @@ static int rt1318_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt1318->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); @@ -686,6 +676,9 @@ static int rt1318_sdw_component_probe(struct snd_soc_component *component)
rt1318->component = component;
+ if (!rt1318->first_hw_init) + return 0; + ret = pm_runtime_resume(component->dev); dev_dbg(&rt1318->sdw_slave->dev, "%s pm_runtime_resume, ret=%d", __func__, ret); if (ret < 0 && ret != -EACCES) @@ -765,8 +758,25 @@ static int rt1318_sdw_init(struct device *dev, struct regmap *regmap, &soc_component_sdw_rt1318, rt1318_sdw_dai, ARRAY_SIZE(rt1318_sdw_dai)); + if (ret < 0) + return ret;
- dev_dbg(&slave->dev, "%s\n", __func__); + /* 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 ret; } @@ -786,10 +796,7 @@ static int rt1318_sdw_probe(struct sdw_slave *slave,
static int rt1318_sdw_remove(struct sdw_slave *slave) { - struct rt1318_sdw_priv *rt1318 = dev_get_drvdata(&slave->dev); - - if (rt1318->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; }
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/rt5682-sdw.c | 37 +++++++++++++++++++++-------------- sound/soc/codecs/rt5682.c | 3 +++ 2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 6726458cf329..dfb702d1b3d4 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -339,7 +339,25 @@ static int rt5682_sdw_init(struct device *dev, struct regmap *regmap, ret = devm_snd_soc_register_component(dev, &rt5682_soc_component_dev, rt5682_dai, ARRAY_SIZE(rt5682_dai)); - dev_dbg(&slave->dev, "%s\n", __func__); + if (ret < 0) + 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 ret; } @@ -361,22 +379,12 @@ static int rt5682_io_init(struct device *dev, struct sdw_slave *slave) regcache_cache_bypass(rt5682->regmap, true);
/* - * 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 */ - if (!rt5682->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(&slave->dev, 3000); - pm_runtime_use_autosuspend(&slave->dev); - + if (!rt5682->first_hw_init) /* 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);
while (loop > 0) { @@ -687,8 +695,7 @@ static int rt5682_sdw_remove(struct sdw_slave *slave) if (rt5682->hw_init) cancel_delayed_work_sync(&rt5682->jack_detect_work);
- if (rt5682->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; } diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c index 5d992543b791..694c581070d9 100644 --- a/sound/soc/codecs/rt5682.c +++ b/sound/soc/codecs/rt5682.c @@ -1017,6 +1017,9 @@ static int rt5682_set_jack_detect(struct snd_soc_component *component,
rt5682->hs_jack = hs_jack;
+ if (rt5682->is_sdw && !rt5682->first_hw_init) + return 0; + if (!hs_jack) { regmap_update_bits(rt5682->regmap, RT5682_IRQ_CTRL_2, RT5682_JD1_EN_MASK, RT5682_JD1_DIS);
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/max98363.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/max98363.c b/sound/soc/codecs/max98363.c index 80a1cb482183..9e3873e56e6d 100644 --- a/sound/soc/codecs/max98363.c +++ b/sound/soc/codecs/max98363.c @@ -165,22 +165,12 @@ static int max98363_io_init(struct sdw_slave *slave) regcache_cache_bypass(max98363->regmap, true);
/* - * 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 */ - if (!max98363->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(dev, 3000); - pm_runtime_use_autosuspend(dev); - + if (!max98363->first_hw_init) /* update count of parent 'active' children */ pm_runtime_set_active(dev);
- /* make sure the device does not suspend immediately */ - pm_runtime_mark_last_busy(dev); - - pm_runtime_enable(dev); - } - pm_runtime_get_noresume(dev);
ret = regmap_read(max98363->regmap, MAX98363_R21FF_REV_ID, ®); @@ -417,10 +407,26 @@ static int max98363_init(struct sdw_slave *slave, struct regmap *regmap) ret = devm_snd_soc_register_component(dev, &soc_codec_dev_max98363, max98363_dai, ARRAY_SIZE(max98363_dai)); - if (ret < 0) + if (ret < 0) { dev_err(dev, "Failed to register codec: %d\n", ret); + 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 + */ + return 0; }
static int max98363_sdw_probe(struct sdw_slave *slave,
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.
The changes are directly inspired by RT711 and RT711-sdca changes.
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 --- sound/soc/codecs/max98373-sdw.c | 40 ++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c index 92d2b872f9f8..b5cb951af570 100644 --- a/sound/soc/codecs/max98373-sdw.c +++ b/sound/soc/codecs/max98373-sdw.c @@ -366,22 +366,12 @@ static int max98373_io_init(struct sdw_slave *slave) regcache_cache_bypass(max98373->regmap, true);
/* - * 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 */ - if (!max98373->first_hw_init) { - /* set autosuspend parameters */ - pm_runtime_set_autosuspend_delay(dev, 3000); - pm_runtime_use_autosuspend(dev); - + if (!max98373->first_hw_init) /* update count of parent 'active' children */ pm_runtime_set_active(dev);
- /* make sure the device does not suspend immediately */ - pm_runtime_mark_last_busy(dev); - - pm_runtime_enable(dev); - } - pm_runtime_get_noresume(dev);
/* Software Reset */ @@ -774,10 +764,27 @@ static int max98373_init(struct sdw_slave *slave, struct regmap *regmap) ret = devm_snd_soc_register_component(dev, &soc_codec_dev_max98373_sdw, max98373_sdw_dai, ARRAY_SIZE(max98373_sdw_dai)); - if (ret < 0) + if (ret < 0) { dev_err(dev, "Failed to register codec: %d\n", ret); + 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 + */ + + return 0; }
static int max98373_update_status(struct sdw_slave *slave, @@ -835,10 +842,7 @@ static int max98373_sdw_probe(struct sdw_slave *slave,
static int max98373_sdw_remove(struct sdw_slave *slave) { - struct max98373_priv *max98373 = dev_get_drvdata(&slave->dev); - - if (max98373->first_hw_init) - pm_runtime_disable(&slave->dev); + pm_runtime_disable(&slave->dev);
return 0; }
On Wed, 02 Aug 2023 10:36:13 -0500, Pierre-Louis Bossart wrote:
This patchset improves the pm_runtime behavior in rare corner cases identified by the Intel CI in the last 6 months.
a) in stress-tests, it's not uncommon to see the following type of warnings when the codec reports as ATTACHED
"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"[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/16] ASoC: SoundWire codecs: return error status in probe commit: 3f3d66ba998fb079c1239430e96e3b138bc63166 [02/16] ASoC: SoundWire codecs: make regmap cache-only in probe commit: 49ae74abc76b2d9be4777e7ac833674fa4749071 [03/16] ASoC: rt5682-sdw: make regmap cache-only in probe commit: 6ab18105029ca3d739dd4c5c18638c7c6d568bbb [04/16] ASoC: rt711: enable pm_runtime in probe, keep status as 'suspended' commit: a8590dd73d9f7fd955ac24a8e210d0721d5c10af [05/16] ASoC: rt711-sdca: enable pm_runtime in probe, keep status as 'suspended' commit: 0c321fb857707ef68ffdb4f9672beb664e6679cc [06/16] ASoC: rt700: enable pm_runtime in probe, keep status as 'suspended' commit: 6b8f8c5e6ffbd431d6cf40584dc8d3027f7e99b3 [07/16] ASoC: rt1712-sdca: enable pm_runtime in probe, keep status as 'suspended' commit: f3da2ed110e2884b29151313eee52947b786e1a7 [08/16] ASoC: rt712-sdca-dmic: enable pm_runtime in probe, keep status as 'suspended' commit: 8d890ecef1ef6e2e81f1709d54f6843846cfd15f [09/16] ASoC: rt715: enable pm_runtime in probe, keep status as 'suspended' commit: 279be5919560d9f3afea1940bfd261297eecfa0c [10/16] ASoC: rt715-sdca: enable pm_runtime in probe, keep status as 'suspended' commit: e4a3b8cf40713d6511391de3286ccba38998756b [11/16] ASoC: rt1308-sdw: enable pm_runtime in probe, keep status as 'suspended' commit: 1772552eb304f3229309f66e2762e835955e2307 [12/16] ASoC: rt1316-sdw: enable pm_runtime in probe, keep status as 'suspended' commit: 64bae6732b2d3364ce4954015c84b4959f7d6e0f [13/16] ASoC: rt1318-sdw: enable pm_runtime in probe, keep status as 'suspended' commit: df93dfa2b4d0f0e2ac2f73f78582a3830659d00e [14/16] ASoC: rt5682-sdw: enable pm_runtime in probe, keep status as 'suspended' commit: 4af11e11defc200439a75a7957b03f3da37e46b2 [15/16] ASoC: max98363: enable pm_runtime in probe, keep status as 'suspended' commit: d6ce285641cfb506b6942818e06270fb091b8fe3 [16/16] ASoC: max98373-sdw: enable pm_runtime in probe, keep status as 'suspended' commit: b48f324f89ab8ee62c3448ef19445a1b292e02d3
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Pierre-Louis Bossart