From: Stefan Binding <sbinding@opensource.cirrus.com>
To: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>
Cc: <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
<patches@opensource.cirrus.com>,
Stefan Binding <sbinding@opensource.cirrus.com>
Subject: [PATCH v1 4/5] ALSA: hda: cs35l41: Remove suspend/resume hda hooks
Date: Tue, 11 Oct 2022 15:35:51 +0100 [thread overview]
Message-ID: <20221011143552.621792-5-sbinding@opensource.cirrus.com> (raw)
In-Reply-To: <20221011143552.621792-1-sbinding@opensource.cirrus.com>
The current code uses calls from the HDA Codec driver to
determine when to suspend/resume by calling hooks via the
hda_component binding.
However, this means the cs35l41 driver relies on the HDA
Codec driver to tell it when to suspend or resume,
creating an additional external dependency, and potentially
creating race conditions in the future. It is better for
the cs35l41 hda driver to decide for itself when the part
should be suspended or resumed.
This makes supporting system suspend easier.
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
sound/pci/hda/cs35l41_hda.c | 31 ++++++++++++-------------------
sound/pci/hda/hda_component.h | 2 --
sound/pci/hda/patch_realtek.c | 19 +------------------
3 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index 102ac4a94a9d6..89f6b4a28d3d7 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -487,10 +487,10 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action)
struct regmap *reg = cs35l41->regmap;
int ret = 0;
- mutex_lock(&cs35l41->fw_mutex);
-
switch (action) {
case HDA_GEN_PCM_ACT_OPEN:
+ pm_runtime_get_sync(dev);
+ mutex_lock(&cs35l41->fw_mutex);
cs35l41->playback_started = true;
if (cs35l41->firmware_running) {
regmap_multi_reg_write(reg, cs35l41_hda_config_dsp,
@@ -508,15 +508,21 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action)
CS35L41_AMP_EN_MASK, 1 << CS35L41_AMP_EN_SHIFT);
if (cs35l41->hw_cfg.bst_type == CS35L41_EXT_BOOST)
regmap_write(reg, CS35L41_GPIO1_CTRL1, 0x00008001);
+ mutex_unlock(&cs35l41->fw_mutex);
break;
case HDA_GEN_PCM_ACT_PREPARE:
+ mutex_lock(&cs35l41->fw_mutex);
ret = cs35l41_global_enable(reg, cs35l41->hw_cfg.bst_type, 1);
+ mutex_unlock(&cs35l41->fw_mutex);
break;
case HDA_GEN_PCM_ACT_CLEANUP:
+ mutex_lock(&cs35l41->fw_mutex);
regmap_multi_reg_write(reg, cs35l41_hda_mute, ARRAY_SIZE(cs35l41_hda_mute));
ret = cs35l41_global_enable(reg, cs35l41->hw_cfg.bst_type, 0);
+ mutex_unlock(&cs35l41->fw_mutex);
break;
case HDA_GEN_PCM_ACT_CLOSE:
+ mutex_lock(&cs35l41->fw_mutex);
ret = regmap_update_bits(reg, CS35L41_PWR_CTRL2,
CS35L41_AMP_EN_MASK, 0 << CS35L41_AMP_EN_SHIFT);
if (cs35l41->hw_cfg.bst_type == CS35L41_EXT_BOOST)
@@ -530,14 +536,16 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action)
}
cs35l41_irq_release(cs35l41);
cs35l41->playback_started = false;
+ mutex_unlock(&cs35l41->fw_mutex);
+
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
break;
default:
dev_warn(cs35l41->dev, "Playback action not supported: %d\n", action);
break;
}
- mutex_unlock(&cs35l41->fw_mutex);
-
if (ret)
dev_err(cs35l41->dev, "Regmap access fail: %d\n", ret);
}
@@ -618,19 +626,6 @@ static int cs35l41_runtime_resume(struct device *dev)
return 0;
}
-static int cs35l41_hda_suspend_hook(struct device *dev)
-{
- dev_dbg(dev, "Request Suspend\n");
- pm_runtime_mark_last_busy(dev);
- return pm_runtime_put_autosuspend(dev);
-}
-
-static int cs35l41_hda_resume_hook(struct device *dev)
-{
- dev_dbg(dev, "Request Resume\n");
- return pm_runtime_get_sync(dev);
-}
-
static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41)
{
int halo_sts;
@@ -863,8 +858,6 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
ret = cs35l41_create_controls(cs35l41);
comps->playback_hook = cs35l41_hda_playback_hook;
- comps->suspend_hook = cs35l41_hda_suspend_hook;
- comps->resume_hook = cs35l41_hda_resume_hook;
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
diff --git a/sound/pci/hda/hda_component.h b/sound/pci/hda/hda_component.h
index 1223621bd62ca..534e845b9cd1d 100644
--- a/sound/pci/hda/hda_component.h
+++ b/sound/pci/hda/hda_component.h
@@ -16,6 +16,4 @@ struct hda_component {
char name[HDA_MAX_NAME_SIZE];
struct hda_codec *codec;
void (*playback_hook)(struct device *dev, int action);
- int (*suspend_hook)(struct device *dev);
- int (*resume_hook)(struct device *dev);
};
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 4b076912bbf4b..e6c4bb5fa041a 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4022,22 +4022,16 @@ static void alc5505_dsp_init(struct hda_codec *codec)
static int alc269_suspend(struct hda_codec *codec)
{
struct alc_spec *spec = codec->spec;
- int i;
if (spec->has_alc5505_dsp)
alc5505_dsp_suspend(codec);
- for (i = 0; i < HDA_MAX_COMPONENTS; i++)
- if (spec->comps[i].suspend_hook)
- spec->comps[i].suspend_hook(spec->comps[i].dev);
-
return alc_suspend(codec);
}
static int alc269_resume(struct hda_codec *codec)
{
struct alc_spec *spec = codec->spec;
- int i;
if (spec->codec_variant == ALC269_TYPE_ALC269VB)
alc269vb_toggle_power_output(codec, 0);
@@ -4068,10 +4062,6 @@ static int alc269_resume(struct hda_codec *codec)
if (spec->has_alc5505_dsp)
alc5505_dsp_resume(codec);
- for (i = 0; i < HDA_MAX_COMPONENTS; i++)
- if (spec->comps[i].resume_hook)
- spec->comps[i].resume_hook(spec->comps[i].dev);
-
return 0;
}
#endif /* CONFIG_PM */
@@ -6664,19 +6654,12 @@ static int comp_bind(struct device *dev)
{
struct hda_codec *cdc = dev_to_hda_codec(dev);
struct alc_spec *spec = cdc->spec;
- int ret, i;
+ int ret;
ret = component_bind_all(dev, spec->comps);
if (ret)
return ret;
- if (snd_hdac_is_power_on(&cdc->core)) {
- codec_dbg(cdc, "Resuming after bind.\n");
- for (i = 0; i < HDA_MAX_COMPONENTS; i++)
- if (spec->comps[i].resume_hook)
- spec->comps[i].resume_hook(spec->comps[i].dev);
- }
-
return 0;
}
--
2.34.1
next prev parent reply other threads:[~2022-10-11 14:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-11 14:35 [PATCH v1 0/5] Support System Suspend and Resume for CS35L41 HDA Stefan Binding
2022-10-11 14:35 ` [PATCH v1 1/5] ALSA: hda: hda_cs_dsp_ctl: Minor clean and redundant code removal Stefan Binding
2022-10-11 14:35 ` [PATCH v1 2/5] ALSA: hda: hda_cs_dsp_ctl: Ensure pwr_lock is held before reading/writing controls Stefan Binding
2022-10-11 14:35 ` [PATCH v1 3/5] ALSA: hda/cs_dsp_ctl: Fix mutex inversion when creating controls Stefan Binding
2022-10-11 14:35 ` Stefan Binding [this message]
2022-10-11 14:35 ` [PATCH v1 5/5] ALSA: hda: cs35l41: Support System Suspend Stefan Binding
2022-10-12 6:12 ` [PATCH v1 0/5] Support System Suspend and Resume for CS35L41 HDA Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221011143552.621792-5-sbinding@opensource.cirrus.com \
--to=sbinding@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox