On 2020/11/2 下午 10:42, Pierre-Louis Bossart wrote:
+struct nau8315_priv {
- struct gpio_desc *enable;
- int enpin_switch;
+};
+static int nau8315_daiops_trigger(struct snd_pcm_substream *substream,
int cmd, struct snd_soc_dai *dai)
+{
- struct snd_soc_component *component = dai->component;
- struct nau8315_priv *nau8315 =
snd_soc_component_get_drvdata(component);
- if (!nau8315->enable)
return 0;
- switch (cmd) {
- case SNDRV_PCM_TRIGGER_START:
- case SNDRV_PCM_TRIGGER_RESUME:
- case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
if (nau8315->enpin_switch) {
gpiod_set_value(nau8315->enable, 1);
dev_dbg(component->dev, "set enable to 1");
}
I know the code is modeled after max98357a.c but I keep wondering if this enpin_switch state is actually useful for anything.
Is there actually a case where the trigger happens before the DAPM_POST_PMU event handled below [1]?
Yes. Currently, I set event on DAPM just with SND_SOC_DAPM_POST_PMU or SND_SOC_DAPM_POST_PMD, so these two specified events will be caught.
If I set SND_SOC_DAPM_PRE_PMU event, it can be caught, too.
break;
- case SNDRV_PCM_TRIGGER_STOP:
- case SNDRV_PCM_TRIGGER_SUSPEND:
- case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
gpiod_set_value(nau8315->enable, 0);
dev_dbg(component->dev, "set enable to 0");
break;
- }
- return 0;
+}
+static int nau8315_enpin_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 nau8315_priv *nau8315 =
snd_soc_component_get_drvdata(component);
[1]
- if (event & SND_SOC_DAPM_POST_PMU)
nau8315->enpin_switch = 1;
- else if (event & SND_SOC_DAPM_POST_PMD)
nau8315->enpin_switch = 0;
And even if this variable was useful, for symmetry should it be PRE_PMU/POST_PMD?
Yes, I agree with your opinion.
From the above description, I suppose you might want to point whether the dapm widget of EN_PIN is redundant, right? That's a reason the physical hardware pin is set to high/low in trigger function point of snd_soc_dai_ops, it always occurred after dapm event. If yes, from my current platform of verification, even if the dapm widget of EN_PIN is removed, the result of sound output is same yet. Maybe the first version, I should submit a simpler version.
- return 0;
+}
+static const struct snd_soc_dapm_widget nau8315_dapm_widgets[] = {
- SND_SOC_DAPM_OUTPUT("Speaker"),
- SND_SOC_DAPM_OUT_DRV_E("EN_Pin", SND_SOC_NOPM, 0, 0, NULL, 0,
nau8315_enpin_event,
SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+};
+static const struct snd_soc_dapm_route nau8315_dapm_routes[] = {
- {"EN_Pin", NULL, "HiFi Playback"},
- {"Speaker", NULL, "EN_Pin"},
+};
+static const struct snd_soc_component_driver nau8315_component_driver = {
- .dapm_widgets = nau8315_dapm_widgets,
- .num_dapm_widgets = ARRAY_SIZE(nau8315_dapm_widgets),
- .dapm_routes = nau8315_dapm_routes,
- .num_dapm_routes = ARRAY_SIZE(nau8315_dapm_routes),
- .idle_bias_on = 1,
- .use_pmdown_time = 1,
is this necessary? This has the side effect of powering-down immediately instead of after a delay.
Yes, I follow most of drivers to use this flag.
- .endianness = 1,
- .non_legacy_dai_naming = 1,
+};
________________________________ ________________________________ The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.