[v3,8/8] ASoC: cs42l42: Wait for debounce interval after resume

Message ID 20230127165111.3010960-9-sbinding@opensource.cirrus.com
State New
Headers
Series ASoC: cs42l42: Add SoundWire support |

Commit Message

Stefan Binding Jan. 27, 2023, 4:51 p.m. UTC
  Since clock stop causes bus reset on Intel controllers, we need
to wait for the debounce interval on resume, to ensure all the
interrupt status registers are set correctly.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/soc/codecs/cs42l42-sdw.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
  

Comments

Pierre-Louis Bossart Jan. 30, 2023, 4:45 p.m. UTC | #1
On 1/27/23 10:51, Stefan Binding wrote:
> Since clock stop causes bus reset on Intel controllers, we need

nit-pick: It's more that the Intel controller has a power optimization
where the context is lost when stopping the clock, which requires a bus
reset and full re-enumeration/initialization when the clock resumes.

The rest of the patch is fine so

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> to wait for the debounce interval on resume, to ensure all the
> interrupt status registers are set correctly.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
  
Richard Fitzgerald Jan. 31, 2023, 11:03 a.m. UTC | #2
On 30/01/2023 16:45, Pierre-Louis Bossart wrote:
> 
> 
> On 1/27/23 10:51, Stefan Binding wrote:
>> Since clock stop causes bus reset on Intel controllers, we need
> 
> nit-pick: It's more that the Intel controller has a power optimization
> where the context is lost when stopping the clock, which requires a bus
> reset and full re-enumeration/initialization when the clock resumes.
> 

Ok, it's true that clock stop doesn't _cause_ bus reset, bus reset is
necessary when exiting clock stop. We can re-word if you want us to
describe that accurately.

But from the codec driver's point of view, a clock stop causes a bus
reset.

> The rest of the patch is fine so
> 
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
>> to wait for the debounce interval on resume, to ensure all the
>> interrupt status registers are set correctly.
>>
>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
  
Pierre-Louis Bossart Jan. 31, 2023, 3:52 p.m. UTC | #3
On 1/31/23 05:03, Richard Fitzgerald wrote:
> On 30/01/2023 16:45, Pierre-Louis Bossart wrote:
>>
>>
>> On 1/27/23 10:51, Stefan Binding wrote:
>>> Since clock stop causes bus reset on Intel controllers, we need
>>
>> nit-pick: It's more that the Intel controller has a power optimization
>> where the context is lost when stopping the clock, which requires a bus
>> reset and full re-enumeration/initialization when the clock resumes.
>>
> 
> Ok, it's true that clock stop doesn't _cause_ bus reset, bus reset is
> necessary when exiting clock stop. We can re-word if you want us to
> describe that accurately.
> 
> But from the codec driver's point of view, a clock stop causes a bus
> reset.

it's fine, we all agree here.

>> The rest of the patch is fine so
>>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>
>>> to wait for the debounce interval on resume, to ensure all the
>>> interrupt status registers are set correctly.
>>>
>>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
  

Patch

diff --git a/sound/soc/codecs/cs42l42-sdw.c b/sound/soc/codecs/cs42l42-sdw.c
index 0de370b40eaf0..79023268d4c1b 100644
--- a/sound/soc/codecs/cs42l42-sdw.c
+++ b/sound/soc/codecs/cs42l42-sdw.c
@@ -447,7 +447,9 @@  static int __maybe_unused cs42l42_sdw_handle_unattach(struct cs42l42_private *cs
 
 static int __maybe_unused cs42l42_sdw_runtime_resume(struct device *dev)
 {
+	static const unsigned int ts_dbnce_ms[] = { 0, 125, 250, 500, 750, 1000, 1250, 1500};
 	struct cs42l42_private *cs42l42 = dev_get_drvdata(dev);
+	unsigned int dbnce;
 	int ret;
 
 	dev_dbg(dev, "Runtime resume\n");
@@ -456,8 +458,14 @@  static int __maybe_unused cs42l42_sdw_runtime_resume(struct device *dev)
 		return 0;
 
 	ret = cs42l42_sdw_handle_unattach(cs42l42);
-	if (ret < 0)
+	if (ret < 0) {
 		return ret;
+	} else if (ret > 0) {
+		dbnce = max(cs42l42->ts_dbnc_rise, cs42l42->ts_dbnc_fall);
+
+		if (dbnce > 0)
+			msleep(ts_dbnce_ms[dbnce]);
+	}
 
 	regcache_cache_only(cs42l42->regmap, false);