[PATCH v3 0/2] ASoC: rt1015p: delay 300ms for waiting calibration

[PATCH v3 0/2] ASoC: rt1015p: delay 300ms for waiting calibration

[Tzung-Bi Shih]

The 1st patch moves SDB control from DAI ops trigger to DAPM event
(per review comments in v1).

The 2nd patch adds the 300ms delay for waiting calibration.

Changes from v2:
- Use gpiod_set_value_cansleep() instead of gpiod_set_value().
(https://patchwork.kernel.org/project/alsa-devel/patch/20201210033617.79300-2-tzungbi@google.com/)
- Assuming the calibration state gets lost after system suspend.
(https://patchwork.kernel.org/project/alsa-devel/patch/20201210033617.79300-3-tzungbi@google.com/)

Changes from v1:
(https://patchwork.kernel.org/project/alsa-devel/patch/20201209033742.3825973-1-tzungbi@google.com/)
- Move the delay from trigger to DAPM event.

Tzung-Bi Shih (2):
  ASoC: rt1015p: move SDB control from trigger to DAPM
  ASoC: rt1015p: delay 300ms after SDB pulling high for calibration

 sound/soc/codecs/rt1015p.c | 69 ++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 37 deletions(-)

-- 
2.29.2.684.gfbc64c5ab5-goog

[PATCH v3 1/2] ASoC: rt1015p: move SDB control from trigger to DAPM

[Tzung-Bi Shih]

Moves SDB control from DAI ops trigger to DAPM.  As long as BCLK
and LRCLK are ready, SDB can be toggled earlier.

Changes from using gpiod_set_value() to gpiod_set_value_cansleep()
because it executes in non-atomic context.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
 sound/soc/codecs/rt1015p.c | 51 ++++++++++----------------------------
 1 file changed, 13 insertions(+), 38 deletions(-)

diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
index 59bb60682270..ee9dfa2dbbf0 100644
--- a/sound/soc/codecs/rt1015p.c
+++ b/sound/soc/codecs/rt1015p.c
@@ -19,60 +19,40 @@
 
 struct rt1015p_priv {
 	struct gpio_desc *sdb;
-	int sdb_switch;
 };
 
-static int rt1015p_daiops_trigger(struct snd_pcm_substream *substream,
-		int cmd, struct snd_soc_dai *dai)
+static int rt1015p_sdb_event(struct snd_soc_dapm_widget *w,
+		struct snd_kcontrol *kcontrol, int event)
 {
-	struct snd_soc_component *component = dai->component;
+	struct snd_soc_component *component =
+		snd_soc_dapm_to_component(w->dapm);
 	struct rt1015p_priv *rt1015p =
 		snd_soc_component_get_drvdata(component);
 
 	if (!rt1015p->sdb)
 		return 0;
 
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (rt1015p->sdb_switch) {
-			gpiod_set_value(rt1015p->sdb, 1);
-			dev_dbg(component->dev, "set sdb to 1");
-		}
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		gpiod_set_value_cansleep(rt1015p->sdb, 1);
+		dev_dbg(component->dev, "set sdb to 1");
 		break;
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		gpiod_set_value(rt1015p->sdb, 0);
+	case SND_SOC_DAPM_POST_PMD:
+		gpiod_set_value_cansleep(rt1015p->sdb, 0);
 		dev_dbg(component->dev, "set sdb to 0");
 		break;
+	default:
+		break;
 	}
 
 	return 0;
 }
 
-static int rt1015p_sdb_event(struct snd_soc_dapm_widget *w,
-		struct snd_kcontrol *kcontrol, int event)
-{
-	struct snd_soc_component *component =
-		snd_soc_dapm_to_component(w->dapm);
-	struct rt1015p_priv *rt1015p =
-		snd_soc_component_get_drvdata(component);
-
-	if (event & SND_SOC_DAPM_POST_PMU)
-		rt1015p->sdb_switch = 1;
-	else if (event & SND_SOC_DAPM_POST_PMD)
-		rt1015p->sdb_switch = 0;
-
-	return 0;
-}
-
 static const struct snd_soc_dapm_widget rt1015p_dapm_widgets[] = {
 	SND_SOC_DAPM_OUTPUT("Speaker"),
 	SND_SOC_DAPM_OUT_DRV_E("SDB", SND_SOC_NOPM, 0, 0, NULL, 0,
 			rt1015p_sdb_event,
-			SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 };
 
 static const struct snd_soc_dapm_route rt1015p_dapm_routes[] = {
@@ -91,10 +71,6 @@ static const struct snd_soc_component_driver rt1015p_component_driver = {
 	.non_legacy_dai_naming	= 1,
 };
 
-static const struct snd_soc_dai_ops rt1015p_dai_ops = {
-	.trigger        = rt1015p_daiops_trigger,
-};
-
 static struct snd_soc_dai_driver rt1015p_dai_driver = {
 	.name = "HiFi",
 	.playback = {
@@ -104,7 +80,6 @@ static struct snd_soc_dai_driver rt1015p_dai_driver = {
 		.channels_min	= 1,
 		.channels_max	= 2,
 	},
-	.ops    = &rt1015p_dai_ops,
 };
 
 static int rt1015p_platform_probe(struct platform_device *pdev)
-- 
2.29.2.684.gfbc64c5ab5-goog

[PATCH v3 2/2] ASoC: rt1015p: delay 300ms after SDB pulling high for calibration

[Tzung-Bi Shih]

RT1015p needs 300ms delay after SDB pulling high for internal
calibration during the power on sequence.

Delays 300ms right before data sends out to avoid data truncated.

Assuming the calibration state gets lost after system suspend.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
 sound/soc/codecs/rt1015p.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/sound/soc/codecs/rt1015p.c b/sound/soc/codecs/rt1015p.c
index ee9dfa2dbbf0..671f2a2130fe 100644
--- a/sound/soc/codecs/rt1015p.c
+++ b/sound/soc/codecs/rt1015p.c
@@ -4,6 +4,7 @@
 //
 // Copyright 2020 The Linux Foundation. All rights reserved.
 
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
@@ -19,6 +20,7 @@
 
 struct rt1015p_priv {
 	struct gpio_desc *sdb;
+	bool calib_done;
 };
 
 static int rt1015p_sdb_event(struct snd_soc_dapm_widget *w,
@@ -36,6 +38,11 @@ static int rt1015p_sdb_event(struct snd_soc_dapm_widget *w,
 	case SND_SOC_DAPM_PRE_PMU:
 		gpiod_set_value_cansleep(rt1015p->sdb, 1);
 		dev_dbg(component->dev, "set sdb to 1");
+
+		if (!rt1015p->calib_done) {
+			msleep(300);
+			rt1015p->calib_done = true;
+		}
 		break;
 	case SND_SOC_DAPM_POST_PMD:
 		gpiod_set_value_cansleep(rt1015p->sdb, 0);
@@ -60,7 +67,20 @@ static const struct snd_soc_dapm_route rt1015p_dapm_routes[] = {
 	{"Speaker", NULL, "SDB"},
 };
 
+#ifdef CONFIG_PM
+static int rt1015p_suspend(struct snd_soc_component *component)
+{
+	struct rt1015p_priv *rt1015p = snd_soc_component_get_drvdata(component);
+
+	rt1015p->calib_done = false;
+	return 0;
+}
+#else
+#define rt1015p_suspend NULL
+#endif
+
 static const struct snd_soc_component_driver rt1015p_component_driver = {
+	.suspend		= rt1015p_suspend,
 	.dapm_widgets		= rt1015p_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(rt1015p_dapm_widgets),
 	.dapm_routes		= rt1015p_dapm_routes,
-- 
2.29.2.684.gfbc64c5ab5-goog

Re: [PATCH v3 0/2] ASoC: rt1015p: delay 300ms for waiting calibration

[Mark Brown]

On Fri, 11 Dec 2020 13:12:22 +0800, Tzung-Bi Shih wrote:
> The 1st patch moves SDB control from DAI ops trigger to DAPM event
> (per review comments in v1).
> 
> The 2nd patch adds the 300ms delay for waiting calibration.
> 
> Changes from v2:
> - Use gpiod_set_value_cansleep() instead of gpiod_set_value().
> (https://patchwork.kernel.org/project/alsa-devel/patch/20201210033617.79300-2-tzungbi@google.com/)
> - Assuming the calibration state gets lost after system suspend.
> (https://patchwork.kernel.org/project/alsa-devel/patch/20201210033617.79300-3-tzungbi@google.com/)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: rt1015p: move SDB control from trigger to DAPM
      commit: 4ab9301710760b99b4229d608eb5599040b2e07e
[2/2] ASoC: rt1015p: delay 300ms after SDB pulling high for calibration
      commit: f102d0d173982be3fc096d0293c1c0245e988ba6

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