* [PATCH] ASoC: wm8741: Remove unneeded startup() callback
@ 2017-01-27 23:35 Sergej Sawazki
2017-01-30 9:22 ` Charles Keepax
0 siblings, 1 reply; 5+ messages in thread
From: Sergej Sawazki @ 2017-01-27 23:35 UTC (permalink / raw)
To: broonie, lgirdwood, ckeepax; +Cc: Sergej Sawazki, alsa-devel, patches
Do not apply rate constraints in the startup() callback. The machine driver
can change the sysclk and hence the supported frame rates in its hw_params().
This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow
master clock switching").
Signed-off-by: Sergej Sawazki <ce3a@gmx.de>
---
sound/soc/codecs/wm8741.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c
index b8c1940..d6e540a 100644
--- a/sound/soc/codecs/wm8741.c
+++ b/sound/soc/codecs/wm8741.c
@@ -176,20 +176,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = {
.list = rates_36864,
};
-static int wm8741_startup(struct snd_pcm_substream *substream,
- struct snd_soc_dai *dai)
-{
- struct snd_soc_codec *codec = dai->codec;
- struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec);
-
- if (wm8741->sysclk)
- snd_pcm_hw_constraint_list(substream->runtime, 0,
- SNDRV_PCM_HW_PARAM_RATE,
- wm8741->sysclk_constraints);
-
- return 0;
-}
-
static int wm8741_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -360,7 +346,6 @@ static int wm8741_set_dai_fmt(struct snd_soc_dai *codec_dai,
SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
static const struct snd_soc_dai_ops wm8741_dai_ops = {
- .startup = wm8741_startup,
.hw_params = wm8741_hw_params,
.set_sysclk = wm8741_set_dai_sysclk,
.set_fmt = wm8741_set_dai_fmt,
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: wm8741: Remove unneeded startup() callback
2017-01-27 23:35 [PATCH] ASoC: wm8741: Remove unneeded startup() callback Sergej Sawazki
@ 2017-01-30 9:22 ` Charles Keepax
2017-01-30 22:56 ` Sergej Sawazki
0 siblings, 1 reply; 5+ messages in thread
From: Charles Keepax @ 2017-01-30 9:22 UTC (permalink / raw)
To: Sergej Sawazki; +Cc: alsa-devel, broonie, lgirdwood, patches
On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote:
> Do not apply rate constraints in the startup() callback. The machine driver
> can change the sysclk and hence the supported frame rates in its hw_params().
> This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow
> master clock switching").
>
> Signed-off-by: Sergej Sawazki <ce3a@gmx.de>
> ---
> sound/soc/codecs/wm8741.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c
> index b8c1940..d6e540a 100644
> --- a/sound/soc/codecs/wm8741.c
> +++ b/sound/soc/codecs/wm8741.c
> @@ -176,20 +176,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = {
> .list = rates_36864,
> };
>
> -static int wm8741_startup(struct snd_pcm_substream *substream,
> - struct snd_soc_dai *dai)
> -{
> - struct snd_soc_codec *codec = dai->codec;
> - struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec);
> -
> - if (wm8741->sysclk)
> - snd_pcm_hw_constraint_list(substream->runtime, 0,
> - SNDRV_PCM_HW_PARAM_RATE,
> - wm8741->sysclk_constraints);
> -
> - return 0;
> -}
> -
This function should not be removed it is performing a useful
function. If a sysclk has already been configured by the machine
driver then we should inform user-space of the rates we can
support from that clock. Without this that information is not
available to user-space, so instead of user-space being able to
resample appropriately it would just error out from hw_params.
Are you perhaps missing a call to clear the sysclk from your
machine driver? Usually a dai_set_sysclk call with a rate of
zero, also I find if it is a machine driver supporting multiple
rates you are best to use ignore_pmdown_time on the DAI link,
assuming the devices don't have long bring up times.
Thanks,
Charles
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: wm8741: Remove unneeded startup() callback
2017-01-30 9:22 ` Charles Keepax
@ 2017-01-30 22:56 ` Sergej Sawazki
2017-01-31 9:49 ` Charles Keepax
0 siblings, 1 reply; 5+ messages in thread
From: Sergej Sawazki @ 2017-01-30 22:56 UTC (permalink / raw)
To: Charles Keepax; +Cc: alsa-devel, broonie, lgirdwood, patches
On Mon, 30 Jan 2017 09:22:59 +0000, Charles Keepax wrote:
> On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote:
>> Do not apply rate constraints in the startup() callback. The machine driver
>> can change the sysclk and hence the supported frame rates in its hw_params().
>> This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow
>> master clock switching").
> This function should not be removed it is performing a useful
> function. If a sysclk has already been configured by the machine
> driver then we should inform user-space of the rates we can
> support from that clock. Without this that information is not
> available to user-space, so instead of user-space being able to
> resample appropriately it would just error out from hw_params.
>
> Are you perhaps missing a call to clear the sysclk from your
> machine driver? Usually a dai_set_sysclk call with a rate of
> zero, also I find if it is a machine driver supporting multiple
> rates you are best to use ignore_pmdown_time on the DAI link,
> assuming the devices don't have long bring up times.
After clearing the sysclk, the codec should 'pretend' to support all
rates, right? (no sysclk -> no rate constrains)
After applying rate constraints using snd_pcm_hw_constraint_list(...),
what would be a clean way to remove the constraints from the rates
parameter?
Thanks,
Sergej
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: wm8741: Remove unneeded startup() callback
2017-01-30 22:56 ` Sergej Sawazki
@ 2017-01-31 9:49 ` Charles Keepax
2017-01-31 20:38 ` Sergej Sawazki
0 siblings, 1 reply; 5+ messages in thread
From: Charles Keepax @ 2017-01-31 9:49 UTC (permalink / raw)
To: Sergej Sawazki; +Cc: alsa-devel, broonie, lgirdwood, patches
On Mon, Jan 30, 2017 at 11:56:52PM +0100, Sergej Sawazki wrote:
> On Mon, 30 Jan 2017 09:22:59 +0000, Charles Keepax wrote:
> > On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote:
> >> Do not apply rate constraints in the startup() callback. The machine driver
> >> can change the sysclk and hence the supported frame rates in its hw_params().
> >> This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow
> >> master clock switching").
>
> > This function should not be removed it is performing a useful
> > function. If a sysclk has already been configured by the machine
> > driver then we should inform user-space of the rates we can
> > support from that clock. Without this that information is not
> > available to user-space, so instead of user-space being able to
> > resample appropriately it would just error out from hw_params.
> >
> > Are you perhaps missing a call to clear the sysclk from your
> > machine driver? Usually a dai_set_sysclk call with a rate of
> > zero, also I find if it is a machine driver supporting multiple
> > rates you are best to use ignore_pmdown_time on the DAI link,
> > assuming the devices don't have long bring up times.
>
> After clearing the sysclk, the codec should 'pretend' to support all
> rates, right? (no sysclk -> no rate constrains)
>
Indeed yes, so a normal pattern would often be setting a sysclk
rate in set_bias_level(on the way up)/hw_params and clearing it in
set_bias_level(on the way down). That way whilst the device is
powered up it is fixed at a certain rate so as not to disrupt any
running audio but whilst powered down you are free to start it up
at any rate.
> After applying rate constraints using snd_pcm_hw_constraint_list(...),
> what would be a clean way to remove the constraints from the rates
> parameter?
>
I am not sure I fully follow here, normally one would close the
stream and open a new one at the new rate. Hence no need to clear
the constraits as its a new stream. Are you intending to keep the
stream open but reconfigure it for new rates?
Thanks,
Charles
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: wm8741: Remove unneeded startup() callback
2017-01-31 9:49 ` Charles Keepax
@ 2017-01-31 20:38 ` Sergej Sawazki
0 siblings, 0 replies; 5+ messages in thread
From: Sergej Sawazki @ 2017-01-31 20:38 UTC (permalink / raw)
To: Charles Keepax; +Cc: alsa-devel, broonie, lgirdwood, patches
On Tue, 31 Jan 2017 09:49:31 +0000, Charles Keepax wrote:
> On Mon, Jan 30, 2017 at 11:56:52PM +0100, Sergej Sawazki wrote:
>> After clearing the sysclk, the codec should 'pretend' to support all
>> rates, right? (no sysclk -> no rate constrains)
> Indeed yes, so a normal pattern would often be setting a sysclk
> rate in set_bias_level(on the way up)/hw_params and clearing it in
> set_bias_level(on the way down). That way whilst the device is
> powered up it is fixed at a certain rate so as not to disrupt any
> running audio but whilst powered down you are free to start it up
> at any rate.
Thanks Charles, it is clear now, sorry for the noise.
Sergej
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-31 20:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-27 23:35 [PATCH] ASoC: wm8741: Remove unneeded startup() callback Sergej Sawazki
2017-01-30 9:22 ` Charles Keepax
2017-01-30 22:56 ` Sergej Sawazki
2017-01-31 9:49 ` Charles Keepax
2017-01-31 20:38 ` Sergej Sawazki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).